All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] git-status: Show empty directories
@ 2012-06-09 19:40 Leila Muhtasib
  2012-06-09 20:13 ` konglu
  2012-06-10  7:15 ` Junio C Hamano
  0 siblings, 2 replies; 17+ messages in thread
From: Leila Muhtasib @ 2012-06-09 19:40 UTC (permalink / raw)
  To: git; +Cc: Leila Muhtasib

git-status now lists empty directories under the untracked header. Before this
modification, git status did not list empty directories. The header changed
from 'Untracked files' to instead display 'Untracked files and directories'.
A helpful reminder is also added after empty directories indicating they cannot
be added/staged if they are empty. git status -u is unchanged, and will still
only show untracked files just as before. As a result, no need for
documentation change.

Empty dirs are work in progress. They result because of one of the following:
user forgot to add files to dir, user forgot to clean up dir, user under
impression dir is staged and will be committed or is already committed. Last
item might occur as some users setup project and dir structures before adding files.

So this patch servers as a helpful reminder to users that they have an empty dir
so they can act upon it. Plus it clarifies git behavior to the user that empty
dirs can't be tracked.

Signed-off-by: Leila Muhtasib <muhtasib@gmail.com>
---

I ran into this issue myself where I thought my dir was already tracked, and 
when I googled and found other people were confused and asking the same question. 
Why don't empty dirs appear under git status when they aren't tracked? So I came 
up with this patch.

In my commit message, I compiled a list of arguments in favor of this patch. 
But I've also thought about arguments against, however I didn't think they were 
compelling enough to not have this patch. So I've also included my own rebuttal 
below :)

1) Direcories can't be tracked, so why show them under untracked?
Because it is a helpful reminder. See reasons in the commit message above for 
how it is helpful. This would make git more user friendly.
Plus untracked does not mean 'it can be tracked', it just means it's not currently 
tracked by git.

2) Empty directories should be ignored
If someone really wants to ignore something, they can put it in the .gitignore 
file. Otherwise, the benefits above out weight this.


 wt-status.c |   24 +++++++++++++++++++-----
 1 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 9ffc535..81bf1aa 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -184,7 +184,12 @@ static void wt_status_print_other_header(struct wt_status *s,
 					 const char *how)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
-	status_printf_ln(s, c, _("%s files:"), what);
+
+	if (s->show_untracked_files == SHOW_NORMAL_UNTRACKED_FILES)
+		status_printf_ln(s, c, _("%s files and directories:"), what);
+	else if (s->show_untracked_files == SHOW_ALL_UNTRACKED_FILES)
+		status_printf_ln(s, c, _("%s files:"), what);
+
 	if (!advice_status_hints)
 		return;
 	status_printf_ln(s, c, _("  (use \"git %s <file>...\" to include in what will be committed)"), how);
@@ -464,16 +469,25 @@ static void wt_status_collect_untracked(struct wt_status *s)
 		return;
 	memset(&dir, 0, sizeof(dir));
 	if (s->show_untracked_files != SHOW_ALL_UNTRACKED_FILES)
-		dir.flags |=
-			DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
+	dir.flags |=
+	  DIR_SHOW_OTHER_DIRECTORIES;
 	setup_standard_excludes(&dir);
 
 	fill_directory(&dir, s->pathspec);
 	for (i = 0; i < dir.nr; i++) {
 		struct dir_entry *ent = dir.entries[i];
 		if (cache_name_is_other(ent->name, ent->len) &&
-		    match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
-			string_list_insert(&s->untracked, ent->name);
+		    match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL)) {
+			if (is_empty_dir(ent->name)) {
+				struct strbuf buf_name = STRBUF_INIT;
+				strbuf_addstr(&buf_name, ent->name);
+				strbuf_addstr(&buf_name, " (empty directories cannot be added)");
+				string_list_insert(&s->untracked, buf_name.buf);
+				strbuf_release(&buf_name);
+			}
+			else
+				string_list_insert(&s->untracked, ent->name);
+		}
 		free(ent);
 	}
 
-- 
1.7.7.5 (Apple Git-26)

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-09 19:40 [PATCH] git-status: Show empty directories Leila Muhtasib
@ 2012-06-09 20:13 ` konglu
  2012-06-09 21:08   ` Leila
  2012-06-10  7:15 ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: konglu @ 2012-06-09 20:13 UTC (permalink / raw)
  To: Leila Muhtasib; +Cc: git


Leila Muhtasib <muhtasib@gmail.com> a écrit :

>  wt-status.c |   24 +++++++++++++++++++-----
>  1 files changed, 19 insertions(+), 5 deletions(-)

Do not forget to also update the test that need 'git
status'. For example, most of the tests in t7508 are
broken with your patch (the change is not huge, just
adding "and directories" at the end of "untracked files:"
here and there and maybe some other minor details).
Otherwise, the idea seems good to me :).

>  	for (i = 0; i < dir.nr; i++) {
>  		struct dir_entry *ent = dir.entries[i];
>  		if (cache_name_is_other(ent->name, ent->len) &&
> -		    match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
> -			string_list_insert(&s->untracked, ent->name);
> +		    match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL)) {
> +			if (is_empty_dir(ent->name)) {
> +				struct strbuf buf_name = STRBUF_INIT;
> +				strbuf_addstr(&buf_name, ent->name);
> +				strbuf_addstr(&buf_name, " (empty directories cannot be added)");
> +				string_list_insert(&s->untracked, buf_name.buf);
> +				strbuf_release(&buf_name);
> +			}
> +			else
> +				string_list_insert(&s->untracked, ent->name);

The structure is
       if (...) {
              /*code*/
       } else {
              /*code*/
       }

Do not forget braces in the "else" part as the firt block needs it.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-09 20:13 ` konglu
@ 2012-06-09 21:08   ` Leila
  2012-06-09 21:14     ` Thomas Rast
  0 siblings, 1 reply; 17+ messages in thread
From: Leila @ 2012-06-09 21:08 UTC (permalink / raw)
  To: konglu; +Cc: git

On Sat, Jun 9, 2012 at 4:13 PM,  <konglu@minatec.inpg.fr> wrote:
>
> Leila Muhtasib <muhtasib@gmail.com> a écrit :
>
>
>>  wt-status.c |   24 +++++++++++++++++++-----
>>  1 files changed, 19 insertions(+), 5 deletions(-)
>
>
> Do not forget to also update the test that need 'git
> status'. For example, most of the tests in t7508 are
> broken with your patch (the change is not huge, just
> adding "and directories" at the end of "untracked files:"
> here and there and maybe some other minor details).
> Otherwise, the idea seems good to me :).
>

Thanks! I didn't update the tests. I will do so now.

>
>>        for (i = 0; i < dir.nr; i++) {
>>                struct dir_entry *ent = dir.entries[i];
>>                if (cache_name_is_other(ent->name, ent->len) &&
>> -                   match_pathspec(s->pathspec, ent->name, ent->len, 0,
>> NULL))
>> -                       string_list_insert(&s->untracked, ent->name);
>> +                   match_pathspec(s->pathspec, ent->name, ent->len, 0,
>> NULL)) {
>> +                       if (is_empty_dir(ent->name)) {
>> +                               struct strbuf buf_name = STRBUF_INIT;
>> +                               strbuf_addstr(&buf_name, ent->name);
>> +                               strbuf_addstr(&buf_name, " (empty
>> directories cannot be added)");
>> +                               string_list_insert(&s->untracked,
>> buf_name.buf);
>> +                               strbuf_release(&buf_name);
>> +                       }
>> +                       else
>> +                               string_list_insert(&s->untracked,
>> ent->name);
>
>
> The structure is
>      if (...) {
>             /*code*/
>      } else {
>             /*code*/
>      }
>
> Do not forget braces in the "else" part as the firt block needs it.

I was under the impression that one liners didn't require parenthesis
according to the style guidelines. I didn't realize that if the 'if'
required it, then the else required it. I will make that change and
remember it for the future. Thanks!

>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-09 21:08   ` Leila
@ 2012-06-09 21:14     ` Thomas Rast
  2012-06-09 21:24       ` Leila
  2012-06-09 21:47       ` konglu
  0 siblings, 2 replies; 17+ messages in thread
From: Thomas Rast @ 2012-06-09 21:14 UTC (permalink / raw)
  To: Leila; +Cc: konglu, git

Leila <muhtasib@gmail.com> writes:

>> The structure is
>>      if (...) {
>>             /*code*/
>>      } else {
>>             /*code*/
>>      }
>>
>> Do not forget braces in the "else" part as the firt block needs it.
>
> I was under the impression that one liners didn't require parenthesis
> according to the style guidelines. I didn't realize that if the 'if'
> required it, then the else required it. I will make that change and
> remember it for the future. Thanks!

It's not required, there's plenty of precedent, even one case within
wt-status.c, of '} else'.  Try running

  git grep '} else$'

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-09 21:14     ` Thomas Rast
@ 2012-06-09 21:24       ` Leila
  2012-06-09 21:47       ` konglu
  1 sibling, 0 replies; 17+ messages in thread
From: Leila @ 2012-06-09 21:24 UTC (permalink / raw)
  To: Thomas Rast; +Cc: konglu, git

On Sat, Jun 9, 2012 at 5:14 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> Leila <muhtasib@gmail.com> writes:
>
>>> The structure is
>>>      if (...) {
>>>             /*code*/
>>>      } else {
>>>             /*code*/
>>>      }
>>>
>>> Do not forget braces in the "else" part as the firt block needs it.
>>
>> I was under the impression that one liners didn't require parenthesis
>> according to the style guidelines. I didn't realize that if the 'if'
>> required it, then the else required it. I will make that change and
>> remember it for the future. Thanks!
>
> It's not required, there's plenty of precedent, even one case within
> wt-status.c, of '} else'.  Try running
>
>  git grep '} else$'
>

I ran the command and was able to see that. Thanks Thomas. I'm fine
following whichever style you guys prefer.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-09 21:14     ` Thomas Rast
  2012-06-09 21:24       ` Leila
@ 2012-06-09 21:47       ` konglu
  2012-06-10  9:01         ` Thomas Rast
  1 sibling, 1 reply; 17+ messages in thread
From: konglu @ 2012-06-09 21:47 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Leila, git


Thomas Rast <trast@student.ethz.ch> a écrit :

> Leila <muhtasib@gmail.com> writes:
>
>>> The structure is
>>>      if (...) {
>>>             /*code*/
>>>      } else {
>>>             /*code*/
>>>      }
>>>
>>> Do not forget braces in the "else" part as the firt block needs it.
>>
>> I was under the impression that one liners didn't require parenthesis
>> according to the style guidelines. I didn't realize that if the 'if'
>> required it, then the else required it. I will make that change and
>> remember it for the future. Thanks!
>
> It's not required, there's plenty of precedent, even one case within
> wt-status.c, of '} else'.  Try running
>
>   git grep '} else$'

It's not because "there's plenty of precedent" that we should not try
to improve the format of the code. That's why there're coding style
rules so that we can keep the improvements consistent.

Thanks.

Lucien Kong

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-09 19:40 [PATCH] git-status: Show empty directories Leila Muhtasib
  2012-06-09 20:13 ` konglu
@ 2012-06-10  7:15 ` Junio C Hamano
  2012-06-10 16:02   ` Leila
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2012-06-10  7:15 UTC (permalink / raw)
  To: Leila Muhtasib; +Cc: git

Leila Muhtasib <muhtasib@gmail.com> writes:

> git-status now lists empty directories under the untracked header. Before this
> modification, git status did not list empty directories. The header changed
> from 'Untracked files' to instead display 'Untracked files and directories'.
> A helpful reminder is also added after empty directories indicating they cannot
> be added/staged if they are empty. git status -u is unchanged, and will still
> only show untracked files just as before. As a result, no need for
> documentation change.

Please do not write a thick wall of text like this.  State the
problem you are trying to solve first, by describing the current
behaviour you want to highlight, and explain why you think the
current behaviour is bad.  Then describe how you propose to solve
that issue in a separate paragraph.

For example:

> git-status now lists empty directories under the untracked header. Before this
> modification, git status did not list empty directories.

The above is backwards.

	"git status" lists untracked files and directories full of
	untracked files, but does not list empty directories.  This
	is bad for such and such reasons.

Then describe your solution (which should be a short two sentence in
this case, because in the problem description you would have justified
adding "empty directories" section).

	Show empty directories to the "Untracked" section as well.
	Because an empty directory by definition does not have
	anything that the user could add, suggest the user to create
	a file to be committed and then add it.

Having said all that, I personally doubt this is a useful change.  I
may thought of adding a README file to a relatively new project that
does not yet have one while in shower but I haven't even created the
file in the working tree.  And I forget about it once I get to the
office.  Should the system remind me to create README and then add?
Your patch would not give me such a reminder once the top-level
directory is populated (because it is no longer empty).  Even if I
were planning to add Documentation/README instead, I would get such
a reminder only if the Documentation directory is empty. Once the
directory is populated, I wouldn't get "create README and then add".
Why should an empty directory so special?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-09 21:47       ` konglu
@ 2012-06-10  9:01         ` Thomas Rast
  2012-06-10  9:46           ` konglu
  2012-06-11 15:08           ` Junio C Hamano
  0 siblings, 2 replies; 17+ messages in thread
From: Thomas Rast @ 2012-06-10  9:01 UTC (permalink / raw)
  To: konglu; +Cc: Leila, git

konglu@minatec.inpg.fr writes:

> Thomas Rast <trast@student.ethz.ch> a écrit :
>
>> Leila <muhtasib@gmail.com> writes:
>>
>>>> The structure is
>>>>      if (...) {
>>>>             /*code*/
>>>>      } else {
>>>>             /*code*/
>>>>      }
>>>>
>>>> Do not forget braces in the "else" part as the firt block needs it.
>>>
>>> I was under the impression that one liners didn't require parenthesis
>>> according to the style guidelines. I didn't realize that if the 'if'
>>> required it, then the else required it. I will make that change and
>>> remember it for the future. Thanks!
>>
>> It's not required, there's plenty of precedent, even one case within
>> wt-status.c, of '} else'.  Try running
>>
>>   git grep '} else$'
>
> It's not because "there's plenty of precedent" that we should not try
> to improve the format of the code. That's why there're coding style
> rules so that we can keep the improvements consistent.

Yeah, and the rules (Documentation/CodingGuidelines) say

 - We avoid using braces unnecessarily.  I.e.

	if (bla) {
		x = 1;
	}

   is frowned upon.  A gray area is when the statement extends
   over a few lines, and/or you have a lengthy comment atop of
   it.  Also, like in the Linux kernel, if there is a long list
   of "else if" statements, it can make sense to add braces to
   single line blocks.

I'm not the one who wrote them, but I'm taking the last sentence to mean
that you should not put the braces unless the omission will break the
vertical alignment of the 'else if' chain.


BTW, there are plenty of cases in git where it is better to stick to the
existing style of the file instead of the CodingGuidelines, unless you
are willing to clean up the file first (and nobody else works on it).

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-10  9:01         ` Thomas Rast
@ 2012-06-10  9:46           ` konglu
  2012-06-10 14:20             ` Leila
  2012-06-11 15:08           ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: konglu @ 2012-06-10  9:46 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Leila, git


Thomas Rast <trast@student.ethz.ch> a écrit :

>  - We avoid using braces unnecessarily.  I.e.
>
> 	if (bla) {
> 		x = 1;
> 	}
>
>    is frowned upon.  A gray area is when the statement extends
>    over a few lines, and/or you have a lengthy comment atop of
>    it.  Also, like in the Linux kernel, if there is a long list
>    of "else if" statements, it can make sense to add braces to
>    single line blocks.
>
> I'm not the one who wrote them, but I'm taking the last sentence to mean
> that you should not put the braces unless the omission will break the
> vertical alignment of the 'else if' chain.

I agree with you and that's what I thought. Still

Junio C Hamano <gitster@pobox.com> a écrit :

> Two points on style (also appear elsewhere in this patch):
>
> 	if (!"applying") {
>  		...
> 	} else {
> 		state->rebase_in_progress = 1;
> 	}
>
>  - "else" comes on the same line as closing "}" of its "if" block;
>
>  - if one of if/else if/else chain has multiple statement block, use {}
>    even for a single statement block in the chain.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-10  9:46           ` konglu
@ 2012-06-10 14:20             ` Leila
  0 siblings, 0 replies; 17+ messages in thread
From: Leila @ 2012-06-10 14:20 UTC (permalink / raw)
  To: konglu; +Cc: Thomas Rast, git

On Sun, Jun 10, 2012 at 5:46 AM,  <konglu@minatec.inpg.fr> wrote:
>
> Thomas Rast <trast@student.ethz.ch> a écrit :
>
>>  - We avoid using braces unnecessarily.  I.e.
>>
>>        if (bla) {
>>                x = 1;
>>        }
>>
>>   is frowned upon.  A gray area is when the statement extends
>>   over a few lines, and/or you have a lengthy comment atop of
>>   it.  Also, like in the Linux kernel, if there is a long list
>>   of "else if" statements, it can make sense to add braces to
>>   single line blocks.
>>
>> I'm not the one who wrote them, but I'm taking the last sentence to mean
>> that you should not put the braces unless the omission will break the
>> vertical alignment of the 'else if' chain.
>
>
> I agree with you and that's what I thought. Still
>
> Junio C Hamano <gitster@pobox.com> a écrit :
>
>> Two points on style (also appear elsewhere in this patch):
>>
>>        if (!"applying") {
>>                ...
>>        } else {
>>                state->rebase_in_progress = 1;
>>        }
>>
>>  - "else" comes on the same line as closing "}" of its "if" block;
>>
>>  - if one of if/else if/else chain has multiple statement block, use {}
>>   even for a single statement block in the chain.
>

I'm happy to produce a patch to update the documentation if there is
consensus to avoid using braces unnecessarily for if statements with
one line, but not in the case that there is {} used in a if/else
chain. Thomas, Lucien, are you on board?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-10  7:15 ` Junio C Hamano
@ 2012-06-10 16:02   ` Leila
  2012-06-10 18:12     ` konglu
  2012-06-11 16:57     ` Junio C Hamano
  0 siblings, 2 replies; 17+ messages in thread
From: Leila @ 2012-06-10 16:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Sun, Jun 10, 2012 at 3:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Leila Muhtasib <muhtasib@gmail.com> writes:
>
>> git-status now lists empty directories under the untracked header. Before this
>> modification, git status did not list empty directories. The header changed
>> from 'Untracked files' to instead display 'Untracked files and directories'.
>> A helpful reminder is also added after empty directories indicating they cannot
>> be added/staged if they are empty. git status -u is unchanged, and will still
>> only show untracked files just as before. As a result, no need for
>> documentation change.
>
> Please do not write a thick wall of text like this.  State the
> problem you are trying to solve first, by describing the current
> behaviour you want to highlight, and explain why you think the
> current behaviour is bad.  Then describe how you propose to solve
> that issue in a separate paragraph.
>
> For example:
>
>> git-status now lists empty directories under the untracked header. Before this
>> modification, git status did not list empty directories.
>
> The above is backwards.
>
>        "git status" lists untracked files and directories full of
>        untracked files, but does not list empty directories.  This
>        is bad for such and such reasons.
>
> Then describe your solution (which should be a short two sentence in
> this case, because in the problem description you would have justified
> adding "empty directories" section).
>
>        Show empty directories to the "Untracked" section as well.
>        Because an empty directory by definition does not have
>        anything that the user could add, suggest the user to create
>        a file to be committed and then add it.

Understood, I'll make sure to do that in the future. I appreciate the example.

>
> Having said all that, I personally doubt this is a useful change.  I
> may thought of adding a README file to a relatively new project that
> does not yet have one while in shower but I haven't even created the
> file in the working tree.  And I forget about it once I get to the
> office.  Should the system remind me to create README and then add?
> Your patch would not give me such a reminder once the top-level
> directory is populated (because it is no longer empty).  Even if I
> were planning to add Documentation/README instead, I would get such
> a reminder only if the Documentation directory is empty. Once the
> directory is populated, I wouldn't get "create README and then add".
> Why should an empty directory so special?
>

So there are two separate discussions here:
1) Should empty dirs be tracked

2) Should empty dirs appear under 'untracked' in git status

The question 'why are empty dirs special', is issue no 1, which I'm
also happy to discuss. The rule empty dirs can't be added in git is
unintuitive, as people use git to keep track of things - all things.
So it's not about an empty dir being special, it's about how people
use git to keep track of things. Now why might people want empty dirs
tracked? 1) People often setup dir structure in projects before adding
any code. They often want to save that state for themselves or to
share it with other team members. 2) Taken from link below: "Although
the directories contain no files, just empty directories, they are
used by the test suite, so I need them to exist in the working tree."
3) It's about saving state, all of it, including empty dirs (even if
they are not useful at the time).

This issue is often raised:
https://git.wiki.kernel.org/index.php/GitFaq#Can_I_add_empty_directories.3F
http://stackoverflow.com/questions/115983/how-do-i-add-an-empty-directory-to-a-git-repository/8944077#8944077
http://stackoverflow.com/questions/9072022/why-cant-i-track-these-files-with-git

I'm aware of the workaround to add a README or .gitignore file to the
empty dir so it can be tracked, but why have a workaround if people
want to add empty dirs. If they want an empty one, they want an empty
one without any files in it. And if they don't want an empty one, they
want to put something real in it (maybe a README, maybe something
else). Thus knowing a dir is empty helps with this point, as the user
can do something about it. Leading our discussion to point 2...

Now, let's put discussion no. 1 aside, and tackle the completely
separate question "should empty dirs appear under 'untracked' in git
status"?

Regardless of whether or not git supports tracking empty dirs, I think
we need this feature. There is confusion among people on this topic
(thus q's on the q&a site). So a helpful message of "empty dirs cannot
be tracked" will abate that. You bring up a good point with having
that message be in the form of a suggestion such as "empty dir: adding
a file to the dir will allow you to track it", "empty dir can't be
tracked: did you forget to add a file?", or something of the sorts.

Why would showing a message about empty dirs in the untracked section help:
1) Clarifies git's position on the matter, and provides a helpful reminder.
2) Abates confusion since ppl may think they've already tracked the
dir (by adding and commit it) and thus it is not appearing under the
untracked list.
3) People use git status to see the state of their working directory.
Feedback of a dir being empty, because they mistakenly added it and
forgot to remove it, or forgot to add something to it is helpful.

And since untracked means not currently tracked by git, and empty dirs
currently at any given point are untracked, they should appear in this
list. So even if you're not sold on discussion no1, I think we need
this patch to solve a communication issue, and to serve as a helpful
reminder which is what git status is all about.

Thanks,
Leila

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-10 16:02   ` Leila
@ 2012-06-10 18:12     ` konglu
  2012-06-10 18:17       ` Leila
  2012-06-11 16:57     ` Junio C Hamano
  1 sibling, 1 reply; 17+ messages in thread
From: konglu @ 2012-06-10 18:12 UTC (permalink / raw)
  To: Leila; +Cc: Junio C Hamano, git


Leila <muhtasib@gmail.com> a écrit :

> The question 'why are empty dirs special', is issue no 1, which I'm
> also happy to discuss. The rule empty dirs can't be added in git is
> unintuitive, as people use git to keep track of things - all things.
> So it's not about an empty dir being special, it's about how people
> use git to keep track of things. Now why might people want empty dirs
> tracked? 1) People often setup dir structure in projects before adding
> any code. They often want to save that state for themselves or to
> share it with other team members.

Yup, that kind of situation is not unusual. Thus, having a reminder by
running 'git status' would help. But sharing empty dir with other team
members is another issue, no ? Even with your implementation, the user
won't be able to share empty directories.

Thanks,

Lucien Kong

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-10 18:12     ` konglu
@ 2012-06-10 18:17       ` Leila
  0 siblings, 0 replies; 17+ messages in thread
From: Leila @ 2012-06-10 18:17 UTC (permalink / raw)
  To: konglu; +Cc: Junio C Hamano, git

On Sun, Jun 10, 2012 at 2:12 PM,  <konglu@minatec.inpg.fr> wrote:
>
> Leila <muhtasib@gmail.com> a écrit :
>
>
>> The question 'why are empty dirs special', is issue no 1, which I'm
>> also happy to discuss. The rule empty dirs can't be added in git is
>> unintuitive, as people use git to keep track of things - all things.
>> So it's not about an empty dir being special, it's about how people
>> use git to keep track of things. Now why might people want empty dirs
>> tracked? 1) People often setup dir structure in projects before adding
>> any code. They often want to save that state for themselves or to
>> share it with other team members.
>
>
> Yup, that kind of situation is not unusual. Thus, having a reminder by
> running 'git status' would help. But sharing empty dir with other team
> members is another issue, no ? Even with your implementation, the user
> won't be able to share empty directories.
>

Yes, there are two issues. I'm advocating for the reminder when
running 'git status'.

Sharing empty dirs with other team members is another issue. My
implementation does not handle this case.

Thanks,
Leila

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-10  9:01         ` Thomas Rast
  2012-06-10  9:46           ` konglu
@ 2012-06-11 15:08           ` Junio C Hamano
  1 sibling, 0 replies; 17+ messages in thread
From: Junio C Hamano @ 2012-06-11 15:08 UTC (permalink / raw)
  To: Thomas Rast; +Cc: konglu, Leila, git

Thomas Rast <trast@student.ethz.ch> writes:

>>> It's not required, there's plenty of precedent, even one case within
>>> wt-status.c, of '} else'.  Try running
>>>
>>>   git grep '} else$'
>>
>> It's not because "there's plenty of precedent" that we should not try
>> to improve the format of the code. That's why there're coding style
>> rules so that we can keep the improvements consistent.
>
> Yeah, and the rules (Documentation/CodingGuidelines) say
>
>  - We avoid using braces unnecessarily.  I.e.
>
> 	if (bla) {
> 		x = 1;
> 	}
>
>    is frowned upon.  A gray area is when the statement extends
>    over a few lines, and/or you have a lengthy comment atop of
>    it.  Also, like in the Linux kernel, if there is a long list
>    of "else if" statements, it can make sense to add braces to
>    single line blocks.
>
> I'm not the one who wrote them, but I'm taking the last sentence to mean
> that you should not put the braces unless the omission will break the
> vertical alignment of the 'else if' chain.

The guidelines are not black-and-white, but the spirit is to suggest
avoiding unnecessary braces around single statement blocks while
allowing exception when consistency across if/else if/... cascade
makes the result easier to read.

> BTW, there are plenty of cases in git where it is better to stick to the
> existing style of the file instead of the CodingGuidelines, unless you
> are willing to clean up the file first (and nobody else works on it).

Yes.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-10 16:02   ` Leila
  2012-06-10 18:12     ` konglu
@ 2012-06-11 16:57     ` Junio C Hamano
  2012-06-11 18:51       ` Leila
  1 sibling, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2012-06-11 16:57 UTC (permalink / raw)
  To: Leila; +Cc: git

Leila <muhtasib@gmail.com> writes:

>> Having said all that, I personally doubt this is a useful change.  I
>> may have thought of adding a README file to a relatively new project that
>> does not yet have one while in shower but I haven't even created the
>> file in the working tree.  And I forget about it once I get to the
>> office.  Should the system remind me to create README and then add?
>> Your patch would not give me such a reminder once the top-level
>> directory is populated (because it is no longer empty).  Even if I
>> were planning to add Documentation/README instead, I would get such
>> a reminder only if the Documentation directory is empty. Once the
>> directory is populated, I wouldn't get "create README and then add".
>> Why should an empty directory so special?
>
> So there are two separate discussions here:
> 1) Should empty dirs be tracked
>
> 2) Should empty dirs appear under 'untracked' in git status

You are not answering my question by asking either these two
questions.

Please read what you quoted again.  I may have forgotten to create
and add README

 - at the toplevel directory; or

 - in the Documentation directory that already has other tracked
   files; or

 - in the Documentation directory that does not have any file yet.

Why do I get a reminder for only the last case?  Also please realize
that at no point in the scenario I am interested in adding an empty
directory.  "How does one add empty directories" is irrelevant to my
question.

A more reasonable answer would have been "the reminder is not about
a yet-to-be-created README file, but is about an empty directory you
might have wanted to place something---there is no way for Git to
guess that you wanted the new file to be README, but at least having
a totally empty directory laying around may be an indication that
you wanted to do something intereseting in it but haven't yet".  If
the proposed commit log message justified the behaviour to treat
only the third one specially that way, I suspect it may make some
sense.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-11 16:57     ` Junio C Hamano
@ 2012-06-11 18:51       ` Leila
  2012-06-11 19:00         ` Leila
  0 siblings, 1 reply; 17+ messages in thread
From: Leila @ 2012-06-11 18:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Jun 11, 2012 at 12:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Leila <muhtasib@gmail.com> writes:
>
>>> Having said all that, I personally doubt this is a useful change.  I
>>> may have thought of adding a README file to a relatively new project that
>>> does not yet have one while in shower but I haven't even created the
>>> file in the working tree.  And I forget about it once I get to the
>>> office.  Should the system remind me to create README and then add?
>>> Your patch would not give me such a reminder once the top-level
>>> directory is populated (because it is no longer empty).  Even if I
>>> were planning to add Documentation/README instead, I would get such
>>> a reminder only if the Documentation directory is empty. Once the
>>> directory is populated, I wouldn't get "create README and then add".
>>> Why should an empty directory so special?
> Please read what you quoted again.  I may have forgotten to create
> and add README
>
>  - at the toplevel directory; or
>
>  - in the Documentation directory that already has other tracked
>   files; or
>
>  - in the Documentation directory that does not have any file yet.
>
> Why do I get a reminder for only the last case?  Also please realize
> that at no point in the scenario I am interested in adding an empty
> directory.  "How does one add empty directories" is irrelevant to my
> question.
>
> A more reasonable answer would have been "the reminder is not about
> a yet-to-be-created README file, but is about an empty directory you
> might have wanted to place something---there is no way for Git to
> guess that you wanted the new file to be README, but at least having
> a totally empty directory laying around may be an indication that
> you wanted to do something intereseting in it but haven't yet".  If
> the proposed commit log message justified the behaviour to treat
> only the third one specially that way, I suspect it may make some
> sense.

I apologize, I misread the scenario and thought you were asking a
different question.

The patch/new implementation I put forth reminds you that you have an
empty dir. The reminder that you have an empty dir  isn't about a
yet-to-be-created README file, but about an empty directory that you
may have wanted to do something interesting in but haven't yet (like
create a README/some other file or delete it even). It servers as
helpful reminder to do something with that empty dir. Is that better?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] git-status: Show empty directories
  2012-06-11 18:51       ` Leila
@ 2012-06-11 19:00         ` Leila
  0 siblings, 0 replies; 17+ messages in thread
From: Leila @ 2012-06-11 19:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Jun 11, 2012 at 2:51 PM, Leila <muhtasib@gmail.com> wrote:
> On Mon, Jun 11, 2012 at 12:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Leila <muhtasib@gmail.com> writes:
>>
>>>> Having said all that, I personally doubt this is a useful change.  I
>>>> may have thought of adding a README file to a relatively new project that
>>>> does not yet have one while in shower but I haven't even created the
>>>> file in the working tree.  And I forget about it once I get to the
>>>> office.  Should the system remind me to create README and then add?
>>>> Your patch would not give me such a reminder once the top-level
>>>> directory is populated (because it is no longer empty).  Even if I
>>>> were planning to add Documentation/README instead, I would get such
>>>> a reminder only if the Documentation directory is empty. Once the
>>>> directory is populated, I wouldn't get "create README and then add".
>>>> Why should an empty directory so special?
>> Please read what you quoted again.  I may have forgotten to create
>> and add README
>>
>>  - at the toplevel directory; or
>>
>>  - in the Documentation directory that already has other tracked
>>   files; or
>>
>>  - in the Documentation directory that does not have any file yet.
>>
>> Why do I get a reminder for only the last case?  Also please realize
>> that at no point in the scenario I am interested in adding an empty
>> directory.  "How does one add empty directories" is irrelevant to my
>> question.
>>
>> A more reasonable answer would have been "the reminder is not about
>> a yet-to-be-created README file, but is about an empty directory you
>> might have wanted to place something---there is no way for Git to
>> guess that you wanted the new file to be README, but at least having
>> a totally empty directory laying around may be an indication that
>> you wanted to do something intereseting in it but haven't yet".  If
>> the proposed commit log message justified the behaviour to treat
>> only the third one specially that way, I suspect it may make some
>> sense.
>
> I apologize, I misread the scenario and thought you were asking a
> different question.
>
> The patch/new implementation I put forth reminds you that you have an
> empty dir. The reminder that you have an empty dir  isn't about a
> yet-to-be-created README file, but about an empty directory that you
> may have wanted to do something interesting in but haven't yet (like
> create a README/some other file or delete it even). It servers as
> helpful reminder to do something with that empty dir. Is that better?

Oh and since it's not about the reminder to add a README file, it
doesn't do anything for the first 2 cases that you mention. Adding a
README is just an example of a use case for this patch.

That being said, I like the idea that you put forth about changing the
wording of the message next to the empty dir to something more
constructive or alert like, so the user can take action regarding the
empty dir.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2012-06-11 19:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-09 19:40 [PATCH] git-status: Show empty directories Leila Muhtasib
2012-06-09 20:13 ` konglu
2012-06-09 21:08   ` Leila
2012-06-09 21:14     ` Thomas Rast
2012-06-09 21:24       ` Leila
2012-06-09 21:47       ` konglu
2012-06-10  9:01         ` Thomas Rast
2012-06-10  9:46           ` konglu
2012-06-10 14:20             ` Leila
2012-06-11 15:08           ` Junio C Hamano
2012-06-10  7:15 ` Junio C Hamano
2012-06-10 16:02   ` Leila
2012-06-10 18:12     ` konglu
2012-06-10 18:17       ` Leila
2012-06-11 16:57     ` Junio C Hamano
2012-06-11 18:51       ` Leila
2012-06-11 19:00         ` Leila

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.