git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Bug in git status
@ 2012-12-26  5:57 Michael Haggerty
  2012-12-26 10:16 ` [PATCH] wt-status: Show ignored files in untracked dirs Antoine Pelisse
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Haggerty @ 2012-12-26  5:57 UTC (permalink / raw)
  To: git discussion list

I think I have found a bug in "git status --untracked-files=all
--ignored", in both 1.8.0 and in master:

$ git init status-test
Initialized empty Git repository in
/home/mhagger/self/proj/git/status-test/.git/
$ cd status-test
$ touch x
$ touch x.ignore-me
$ mkdir y
$ touch y/foo
$ touch y/foo.ignore-me
$ git status --porcelain --untracked-files=all --ignored
?? x
?? x.ignore-me
?? y/foo
?? y/foo.ignore-me

The above output is what I expect.  But if I add a .gitignore file, the
output of y/foo.ignore-me is incorrectly suppressed:

$ echo '*.ignore-me' >.gitignore
$ git status --porcelain --untracked-files=all --ignored
?? .gitignore
?? x
?? y/foo
!! x.ignore-me

I came across this problem when trying to use the results of the above
command to build a more flexible "git clean" type of script.

I don't have time to look into this at the moment, if somebody wants to
jump in.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

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

* [PATCH] wt-status: Show ignored files in untracked dirs
  2012-12-26  5:57 Bug in git status Michael Haggerty
@ 2012-12-26 10:16 ` Antoine Pelisse
  2012-12-26 13:31   ` [PATCH v2] " Antoine Pelisse
  0 siblings, 1 reply; 16+ messages in thread
From: Antoine Pelisse @ 2012-12-26 10:16 UTC (permalink / raw)
  To: git; +Cc: Antoine Pelisse

When looking for ignored files, we do not recurse into untracked
directory, and simply consider the directory ignored status.
As a consequence, we don't see ignored files in those directories.

Change that behavior by recursing into untracked directories searching
for ignored files.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
I jumped in.

This seems to be broken since the creation of the --ignored option to
wt-status.

This fixes the issue and breaks none of the existing tests.
The behavior seems sane to me, giving something like that:

  ?? .gitignore
  ?? x
  ?? y/foo
  !! x.ignore-me
  !! y/foo.ignore-me

Cheers,
Antoine

 wt-status.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index 2a9658b..7c41488 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -516,7 +516,7 @@ static void wt_status_collect_untracked(struct wt_status *s)

 	if (s->show_ignored_files) {
 		dir.nr = 0;
-		dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
+		dir.flags = DIR_SHOW_IGNORED;
 		fill_directory(&dir, s->pathspec);
 		for (i = 0; i < dir.nr; i++) {
 			struct dir_entry *ent = dir.entries[i];
--
1.8.1.rc3.11.g86c3e6e.dirty

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

* [PATCH v2] wt-status: Show ignored files in untracked dirs
  2012-12-26 10:16 ` [PATCH] wt-status: Show ignored files in untracked dirs Antoine Pelisse
@ 2012-12-26 13:31   ` Antoine Pelisse
  2012-12-27  2:37     ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Antoine Pelisse @ 2012-12-26 13:31 UTC (permalink / raw)
  To: git; +Cc: Antoine Pelisse

When looking for ignored files, we do not recurse into untracked
directory, and simply consider the directory ignored status.
As a consequence, we don't see ignored files in those directories.

Change that behavior by recursing into untracked directories, if not
ignored themselves, searching for ignored files.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
Actually, the previous patch breaks the case where the directory is ignored.
This one should fix both issues.
Let me know if you see any other use case that could be an issue.

 dir.c       | 7 +++++++
 wt-status.c | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/dir.c b/dir.c
index 5a83aa7..2863799 100644
--- a/dir.c
+++ b/dir.c
@@ -1042,6 +1042,13 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 			return path_ignored;
 	}

+	/*
+	 * Don't recurse into ignored directories when looking for
+	 * ignored files, but still show the directory as ignored.
+	 */
+	if (exclude && (dir->flags & DIR_SHOW_IGNORED) && dtype == DT_DIR)
+		return path_handled;
+
 	switch (dtype) {
 	default:
 		return path_ignored;
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..7c41488 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -516,7 +516,7 @@ static void wt_status_collect_untracked(struct wt_status *s)

 	if (s->show_ignored_files) {
 		dir.nr = 0;
-		dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
+		dir.flags = DIR_SHOW_IGNORED;
 		fill_directory(&dir, s->pathspec);
 		for (i = 0; i < dir.nr; i++) {
 			struct dir_entry *ent = dir.entries[i];
--
1.8.1.rc3.12.g8864e38

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

* Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
  2012-12-26 13:31   ` [PATCH v2] " Antoine Pelisse
@ 2012-12-27  2:37     ` Junio C Hamano
  2012-12-27  3:48       ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2012-12-27  2:37 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: git

Antoine Pelisse <apelisse@gmail.com> writes:

> When looking for ignored files, we do not recurse into untracked
> directory, and simply consider the directory ignored status.

When asked to show ignored ones, instead of listing all ignored
files in such a directory, we just say "everything in this directory
is ignored"?

That sounds like a more desirable behaviour, than listing everything
there, at least to me, but perhaps I am missing something.

Care to add a test for this new behaviour?

> As a consequence, we don't see ignored files in those directories.
>
> Change that behavior by recursing into untracked directories, if not
> ignored themselves, searching for ignored files.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
> Actually, the previous patch breaks the case where the directory is ignored.
> This one should fix both issues.
> Let me know if you see any other use case that could be an issue.
>
>  dir.c       | 7 +++++++
>  wt-status.c | 2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/dir.c b/dir.c
> index 5a83aa7..2863799 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1042,6 +1042,13 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
>  			return path_ignored;
>  	}
>
> +	/*
> +	 * Don't recurse into ignored directories when looking for
> +	 * ignored files, but still show the directory as ignored.
> +	 */
> +	if (exclude && (dir->flags & DIR_SHOW_IGNORED) && dtype == DT_DIR)
> +		return path_handled;
> +
>  	switch (dtype) {
>  	default:
>  		return path_ignored;
> diff --git a/wt-status.c b/wt-status.c
> index 2a9658b..7c41488 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -516,7 +516,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
>
>  	if (s->show_ignored_files) {
>  		dir.nr = 0;
> -		dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
> +		dir.flags = DIR_SHOW_IGNORED;
>  		fill_directory(&dir, s->pathspec);
>  		for (i = 0; i < dir.nr; i++) {
>  			struct dir_entry *ent = dir.entries[i];
> --
> 1.8.1.rc3.12.g8864e38

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

* Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
  2012-12-27  2:37     ` Junio C Hamano
@ 2012-12-27  3:48       ` Jeff King
  2012-12-27  5:08         ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2012-12-27  3:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git

On Wed, Dec 26, 2012 at 06:37:55PM -0800, Junio C Hamano wrote:

> Antoine Pelisse <apelisse@gmail.com> writes:
> 
> > When looking for ignored files, we do not recurse into untracked
> > directory, and simply consider the directory ignored status.
> 
> When asked to show ignored ones, instead of listing all ignored
> files in such a directory, we just say "everything in this directory
> is ignored"?
> 
> That sounds like a more desirable behaviour, than listing everything
> there, at least to me, but perhaps I am missing something.

I do not use this feature myself, but I would think that it should
respect the same DIR_SHOW_OTHER_DIRECTORIES flag (or a parallel flag)
that we already hook into "--untracked={all,normal}".

IOW, given:

  git init
  mkdir untracked ignored
  >untracked/file
  >ignored/file
  echo ignored >.git/info/exclude

I would expect:

  $ git status --short --ignored --untracked=normal
  ?? untracked/
  !! ignored/

  $ git status --short --ignored --untracked=all
  ?? untracked/file
  !! ignored/file

I do not know if anybody cares about the distinction, but optionally we
could give --ignored its own selector, like:

  $ git status --short --ignored=all --untracked=normal
  ?? untracked/
  !! ignored/file

where obviously it would default to "none" (whereas untracked defaults
to "normal"). But the behavior with Antoine's patch is:

  $ git status --short --ignored --untracked=normal
  ?? untracked/
  !! ignored

  $ git status --short --ignored --untracked=all
  ?? untracked/file
  !! ignored

which seems wrong to me for two reasons:

  1. It does not recurse for ignored but untracked entries. Neither does
     the current code, but I think it should.

  2. It loses the trailing slash from the ignored directory in both
     cases (which is printed by the current code).

-Peff

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

* Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
  2012-12-27  3:48       ` Jeff King
@ 2012-12-27  5:08         ` Junio C Hamano
  2012-12-27 16:14           ` Antoine Pelisse
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2012-12-27  5:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Antoine Pelisse, git

Jeff King <peff@peff.net> writes:

> IOW, given:
>
>   git init
>   mkdir untracked ignored
>   >untracked/file
>   >ignored/file
>   echo ignored >.git/info/exclude
>
> I would expect:
>
>   $ git status --short --ignored --untracked=normal
>   ?? untracked/
>   !! ignored/

Sensible.

>   $ git status --short --ignored --untracked=all
>   ?? untracked/file
>   !! ignored/file

Again sensible; OK, --untracked=all is what I was missing.

> I do not know if anybody cares about the distinction, but optionally we
> could give --ignored its own selector, like:
>
>   $ git status --short --ignored=all --untracked=normal
>   ?? untracked/
>   !! ignored/file
>
> where obviously it would default to "none" (whereas untracked defaults
> to "normal").

We could just say the selector for the ignored implicitly follows
what is given for --untracked, if we don't care.

> But the behavior with Antoine's patch is:
>
>   $ git status --short --ignored --untracked=normal
>   ?? untracked/
>   !! ignored
>
>   $ git status --short --ignored --untracked=all
>   ?? untracked/file
>   !! ignored
>
> which seems wrong to me for two reasons:
>
>   1. It does not recurse for ignored but untracked entries. Neither does
>      the current code, but I think it should.
>
>   2. It loses the trailing slash from the ignored directory in both
>      cases (which is printed by the current code).

Nicely analysed.  Perhaps we would want new test pieces to define
the behaviour we want to see first?

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

* Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
  2012-12-27  5:08         ` Junio C Hamano
@ 2012-12-27 16:14           ` Antoine Pelisse
  2012-12-27 16:19             ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Antoine Pelisse @ 2012-12-27 16:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git

> Nicely analysed.  Perhaps we would want new test pieces to define
> the behaviour we want to see first?

I think we should.

I also thought about the use case of "committed" and ignored directory
which is also broken to me (point 3 in the table below).

Anyway I tried to make a table to sum-up/discuss the list of behaviors
we would like to see/test, taking Jeff mail into account.
(warning: that requires fixed width font)

|----------------------+---------------------+---------------------------|
| Output               | A. status --ignored | B. status --ignored -uall |
|                      |                     | (or with potential        |
|                      |                     | --ignored=all)            |
|----------------------+---------------------+---------------------------|
| 1. Untracked dirU    | Current:            | Current:                  |
| with ignored unco.ig | Empty               | Empty                     |
| in it                |                     |                           |
|                      | Expected:           | Expected:                 |
|                      | !!dirU/             | !!dirU/unco.ig            |
|----------------------+---------------------+---------------------------|
| 2. Untracked and     | Current (OK):       | Current:                  |
| ignored dirU with    | !!dirU/             | !!dirU/                   |
| file in it           |                     |                           |
|                      |                     | Expected:                 |
|                      |                     | !!dirU/unco               |
|----------------------+---------------------+---------------------------|
| 3. "Committed" dirA  | Current:            | Current:                  |
| yet ignored          | Empty               | Empty                     |
| with uncommitted     |                     |                           |
| file in it           | Expected:           | Expected:                 |
|                      | dirA/               | dirA/unco                 |
|----------------------+---------------------+---------------------------|

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

* Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
  2012-12-27 16:14           ` Antoine Pelisse
@ 2012-12-27 16:19             ` Jeff King
  2012-12-27 17:35               ` Antoine Pelisse
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2012-12-27 16:19 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: Junio C Hamano, git

On Thu, Dec 27, 2012 at 05:14:54PM +0100, Antoine Pelisse wrote:

> > Nicely analysed.  Perhaps we would want new test pieces to define
> > the behaviour we want to see first?
> 
> I think we should.
> 
> I also thought about the use case of "committed" and ignored directory
> which is also broken to me (point 3 in the table below).

By "committed", I assume you meat that you have "dirA/unco" as an
untracked file, and "dirA/committed" as a file in the index?

> Anyway I tried to make a table to sum-up/discuss the list of behaviors
> we would like to see/test, taking Jeff mail into account.
> (warning: that requires fixed width font)
> 
> |----------------------+---------------------+---------------------------|
> | Output               | A. status --ignored | B. status --ignored -uall |
> |                      |                     | (or with potential        |
> |                      |                     | --ignored=all)            |
> |----------------------+---------------------+---------------------------|
> | 1. Untracked dirU    | Current:            | Current:                  |
> | with ignored unco.ig | Empty               | Empty                     |
> | in it                |                     |                           |
> |                      | Expected:           | Expected:                 |
> |                      | !!dirU/             | !!dirU/unco.ig            |
> |----------------------+---------------------+---------------------------|
> | 2. Untracked and     | Current (OK):       | Current:                  |
> | ignored dirU with    | !!dirU/             | !!dirU/                   |
> | file in it           |                     |                           |
> |                      |                     | Expected:                 |
> |                      |                     | !!dirU/unco               |
> |----------------------+---------------------+---------------------------|
> | 3. "Committed" dirA  | Current:            | Current:                  |
> | yet ignored          | Empty               | Empty                     |
> | with uncommitted     |                     |                           |
> | file in it           | Expected:           | Expected:                 |
> |                      | dirA/               | dirA/unco                 |
> |----------------------+---------------------+---------------------------|

Thanks for putting this together. I agree with the expected output in
each case, and I think this covers the cases we have seen (case 1 is
Michael's original report, case 2 is what I wrote in my mail, and case 3
is the one you just came up with). I can't think offhand of any others.

-Peff

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

* Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
  2012-12-27 16:19             ` Jeff King
@ 2012-12-27 17:35               ` Antoine Pelisse
  2012-12-28 14:05                 ` Antoine Pelisse
  0 siblings, 1 reply; 16+ messages in thread
From: Antoine Pelisse @ 2012-12-27 17:35 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

> By "committed", I assume you meat that you have "dirA/unco" as an
> untracked file, and "dirA/committed" as a file in the index?

Of course,

> Thanks for putting this together. I agree with the expected output in
> each case, and I think this covers the cases we have seen (case 1 is
> Michael's original report, case 2 is what I wrote in my mail, and case 3
> is the one you just came up with). I can't think offhand of any others.

Great, so I can build some tests reflecting those behaviors while
waiting more inputs

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

* Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
  2012-12-27 17:35               ` Antoine Pelisse
@ 2012-12-28 14:05                 ` Antoine Pelisse
  2012-12-29  7:22                   ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Antoine Pelisse @ 2012-12-28 14:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

Hey Peff,
I actually have an issue with the behavior we discussed (referenced as 1.A.)

Using the example from Michael's mail, I end up having this:
$ git status --porcelain --ignored
?? .gitignore
?? x
?? y/
!! x.ignore-me
!! y/

y/ is referred as untracked, because it contains untracked files, and
then as ignored because it
contains ignored files.

Showing it twice doesn't feel right though, so I guess we should only
show "?? y/" with untracked=normal,
and "!! y/foo.ignore-me" when using untracked=all

What do you think ?




On Thu, Dec 27, 2012 at 6:35 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>> By "committed", I assume you meat that you have "dirA/unco" as an
>> untracked file, and "dirA/committed" as a file in the index?
>
> Of course,
>
>> Thanks for putting this together. I agree with the expected output in
>> each case, and I think this covers the cases we have seen (case 1 is
>> Michael's original report, case 2 is what I wrote in my mail, and case 3
>> is the one you just came up with). I can't think offhand of any others.
>
> Great, so I can build some tests reflecting those behaviors while
> waiting more inputs

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

* Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
  2012-12-28 14:05                 ` Antoine Pelisse
@ 2012-12-29  7:22                   ` Jeff King
  2012-12-30 14:39                     ` [PATCH 1/2] dir.c: Make git-status --ignored more consistent Antoine Pelisse
  2012-12-30 14:39                     ` [PATCH 2/2] git-status: Test --ignored behavior Antoine Pelisse
  0 siblings, 2 replies; 16+ messages in thread
From: Jeff King @ 2012-12-29  7:22 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: Junio C Hamano, git

On Fri, Dec 28, 2012 at 03:05:30PM +0100, Antoine Pelisse wrote:

> Using the example from Michael's mail, I end up having this:
> $ git status --porcelain --ignored
> ?? .gitignore
> ?? x
> ?? y/
> !! x.ignore-me
> !! y/
> 
> y/ is referred as untracked, because it contains untracked files, and
> then as ignored because it
> contains ignored files.
> 
> Showing it twice doesn't feel right though, so I guess we should only
> show "?? y/" with untracked=normal,
> and "!! y/foo.ignore-me" when using untracked=all
> 
> What do you think ?

Good catch. I agree that showing just "?? y/" in the untracked=normal
case makes sense. It makes the definition of "!!" to mean "all untracked
files in this path are ignored". IOW, showing "??" means there are one
or more untracked, unignored files. There may _also_ be ignored files,
but we do not say (nor we even necessarily need to bother checking).

In retrospect, I think it might have made more sense to use the second
character of an untracked line to represent "ignored". That is, the
output:

  ?? .gitignore
  ?? x
  ?! y/
  !! x.ignore-me

would make sense to me. But that would be a backwards-incompatible
change at this point, and I don't think it's worth it.

-Peff

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

* [PATCH 1/2] dir.c: Make git-status --ignored more consistent
  2012-12-29  7:22                   ` Jeff King
@ 2012-12-30 14:39                     ` Antoine Pelisse
  2012-12-30 14:54                       ` Antoine Pelisse
  2012-12-30 14:39                     ` [PATCH 2/2] git-status: Test --ignored behavior Antoine Pelisse
  1 sibling, 1 reply; 16+ messages in thread
From: Antoine Pelisse @ 2012-12-30 14:39 UTC (permalink / raw)
  To: Jeff King, Junio C Hamano; +Cc: Antoine Pelisse, git

The current behavior of git-status is inconsistent and
misleading. Especially when used with --untracked-files=all option:

 - files ignored in untracked directories will be missing from status
 output.
 - untracked files in committed yet ignored directories are also
 missing.
 - with --untracked-files=normal, untracked directories that contains
 only ignored files are dropped too.

Make the behavior more consistent across all possible use cases:

 - "--ignored --untracked-files=normal" doesn't show each specific
 files but top directory.
 Shows untracked directories that only contains ignored files, and
 ignored tracked directories with untracked files.
 - "--ignored --untracked-files=all" shows all ignored files, either
 because it's in an ignored directory (tracked or untracked), or
 because the file is explicitly ignored.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 dir.c       |   98 +++++++++++++++++++++++++++++++++++++++++++++++------------
 wt-status.c |    4 ++-
 2 files changed, 81 insertions(+), 21 deletions(-)

diff --git a/dir.c b/dir.c
index 5a83aa7..d0c92dc 100644
--- a/dir.c
+++ b/dir.c
@@ -834,8 +834,9 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len)
  * traversal routine.
  *
  * Case 1: If we *already* have entries in the index under that
- * directory name, we always recurse into the directory to see
- * all the files.
+ * directory name, we recurse into the directory to see all the files,
+ * unless the directory is excluded and we want to show ignored
+ * directories
  *
  * Case 2: If we *already* have that directory name as a gitlink,
  * we always continue to see it as a gitlink, regardless of whether
@@ -849,6 +850,9 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len)
  *      just a directory, unless "hide_empty_directories" is
  *      also true and the directory is empty, in which case
  *      we just ignore it entirely.
+ *      if we are looking for ignored directories, look if it
+ *      contains only ignored files to decide if it must be shown as
+ *      ignored or not.
  *  (b) if it looks like a git directory, and we don't have
  *      'no_gitlinks' set we treat it as a gitlink, and show it
  *      as a directory.
@@ -861,12 +865,15 @@ enum directory_treatment {
 };
 
 static enum directory_treatment treat_directory(struct dir_struct *dir,
-	const char *dirname, int len,
+	const char *dirname, int len, int exclude,
 	const struct path_simplify *simplify)
 {
 	/* The "len-1" is to strip the final '/' */
 	switch (directory_exists_in_index(dirname, len-1)) {
 	case index_directory:
+		if ((dir->flags & DIR_SHOW_OTHER_DIRECTORIES) && exclude)
+			break;
+
 		return recurse_into_directory;
 
 	case index_gitdir:
@@ -886,7 +893,23 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
 	}
 
 	/* This is the "show_other_directories" case */
-	if (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))
+
+	/*
+	 * We are looking for ignored files and our directory is not ignored,
+	 * check if it contains only ignored files
+	 */
+	if ((dir->flags & DIR_SHOW_IGNORED) && !exclude) {
+		int ignored;
+		dir->flags &= ~DIR_SHOW_IGNORED;
+		dir->flags |= DIR_HIDE_EMPTY_DIRECTORIES;
+		ignored = read_directory_recursive(dir, dirname, len, 1, simplify);
+		dir->flags &= ~DIR_HIDE_EMPTY_DIRECTORIES;
+		dir->flags |= DIR_SHOW_IGNORED;
+
+		return ignored ? ignore_directory : show_directory;
+	}
+	if (!(dir->flags & DIR_SHOW_IGNORED) &&
+	    !(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))
 		return show_directory;
 	if (!read_directory_recursive(dir, dirname, len, 1, simplify))
 		return ignore_directory;
@@ -894,6 +917,49 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
 }
 
 /*
+ * Decide what to do when we find a file while traversing the
+ * filesystem. Mostly two cases:
+ *
+ *  1. We are looking for ignored files
+ *   (a) File is ignored, include it
+ *   (b) File is in ignored path, include it
+ *   (c) File is not ignored, exclude it
+ *
+ *  2. Other scenarios, include the file if not excluded
+ *
+ * Return 1 for exclude, 0 for include.
+ */
+static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude, int *dtype)
+{
+	struct path_exclude_check check;
+	int exclude_file = 0;
+
+	if (exclude)
+		exclude_file = !(dir->flags & DIR_SHOW_IGNORED);
+	else if (dir->flags & DIR_SHOW_IGNORED) {
+		/*
+		 * Optimization:
+		 * Don't spend time on indexed files, they won't be
+		 * added to the list anyway
+		 */
+		struct cache_entry *ce = index_name_exists(&the_index,
+		    path->buf, path->len, ignore_case);
+
+		if (ce)
+			return 1;
+
+		path_exclude_check_init(&check, dir);
+
+		if (!path_excluded(&check, path->buf, path->len, dtype))
+			exclude_file = 1;
+
+		path_exclude_check_clear(&check);
+	}
+
+	return exclude_file;
+}
+
+/*
  * This is an inexact early pruning of any recursive directory
  * reading - if the path cannot possibly be in the pathspec,
  * return true, and we'll skip it early.
@@ -1031,27 +1097,14 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 	if (dtype == DT_UNKNOWN)
 		dtype = get_dtype(de, path->buf, path->len);
 
-	/*
-	 * Do we want to see just the ignored files?
-	 * We still need to recurse into directories,
-	 * even if we don't ignore them, since the
-	 * directory may contain files that we do..
-	 */
-	if (!exclude && (dir->flags & DIR_SHOW_IGNORED)) {
-		if (dtype != DT_DIR)
-			return path_ignored;
-	}
-
 	switch (dtype) {
 	default:
 		return path_ignored;
 	case DT_DIR:
 		strbuf_addch(path, '/');
-		switch (treat_directory(dir, path->buf, path->len, simplify)) {
+
+		switch (treat_directory(dir, path->buf, path->len, exclude, simplify)) {
 		case show_directory:
-			if (exclude != !!(dir->flags
-					  & DIR_SHOW_IGNORED))
-				return path_ignored;
 			break;
 		case recurse_into_directory:
 			return path_recurse;
@@ -1061,7 +1114,12 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 		break;
 	case DT_REG:
 	case DT_LNK:
-		break;
+		switch(treat_file(dir, path, exclude, &dtype)) {
+		case 1:
+			return path_ignored;
+		default:
+			break;
+		}
 	}
 	return path_handled;
 }
diff --git a/wt-status.c b/wt-status.c
index 2a9658b..d7cfe8f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -516,7 +516,9 @@ static void wt_status_collect_untracked(struct wt_status *s)
 
 	if (s->show_ignored_files) {
 		dir.nr = 0;
-		dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
+		dir.flags = DIR_SHOW_IGNORED;
+		if (s->show_untracked_files != SHOW_ALL_UNTRACKED_FILES)
+			dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
 		fill_directory(&dir, s->pathspec);
 		for (i = 0; i < dir.nr; i++) {
 			struct dir_entry *ent = dir.entries[i];
-- 
1.7.9.5

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

* [PATCH 2/2] git-status: Test --ignored behavior
  2012-12-29  7:22                   ` Jeff King
  2012-12-30 14:39                     ` [PATCH 1/2] dir.c: Make git-status --ignored more consistent Antoine Pelisse
@ 2012-12-30 14:39                     ` Antoine Pelisse
  1 sibling, 0 replies; 16+ messages in thread
From: Antoine Pelisse @ 2012-12-30 14:39 UTC (permalink / raw)
  To: Jeff King, Junio C Hamano; +Cc: Antoine Pelisse, git

Test all possible use-cases of git-status --ignored with
--untracked-files to normal and all:

 - untracked directory is listed as untracked if it has a mix of
 untracked and ignored files in it.
 with -uall, ignored/untracked files are listed as
 ignored/untracked.

 - untracked directory with only ignored files is listed as ignored.
 with -uall, all files in the directory are listed.

 - ignored directory is listed as ignored. With -uall, all files in
 the directory are listed as ignored.

 - ignored and committed directory is listed as ignored if it has
 untracked files.
 with -uall, all untracked files in the directory are listed as
 ignored.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 t/t7061-wtstatus-ignore.sh |  146 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 146 insertions(+)
 create mode 100755 t/t7061-wtstatus-ignore.sh

diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
new file mode 100755
index 0000000..0da1214
--- /dev/null
+++ b/t/t7061-wtstatus-ignore.sh
@@ -0,0 +1,146 @@
+#!/bin/sh
+
+test_description='git-status ignored files'
+
+. ./test-lib.sh
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+?? untracked/
+EOF
+
+test_expect_success 'status untracked directory with --ignored' '
+	echo "ignored" >.gitignore &&
+	mkdir untracked &&
+	: >untracked/ignored &&
+	: >untracked/uncommitted &&
+	git status --porcelain --ignored >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+?? untracked/uncommitted
+!! untracked/ignored
+EOF
+
+test_expect_success 'status untracked directory with --ignored -u' '
+	git status --porcelain --ignored -u >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! ignored/
+EOF
+
+test_expect_success 'status ignored directory with --ignore' '
+	rm -rf untracked &&
+	mkdir ignored &&
+	: >ignored/uncommitted &&
+	git status --porcelain --ignored >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! ignored/uncommitted
+EOF
+
+test_expect_success 'status ignored directory with --ignore -u' '
+	git status --porcelain --ignored -u >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! untracked-ignored/
+EOF
+
+test_expect_success 'status untracked directory with ignored files with --ignore' '
+	rm -rf ignored &&
+	mkdir untracked-ignored &&
+	mkdir untracked-ignored/test &&
+	: >untracked-ignored/ignored &&
+	: >untracked-ignored/test/ignored &&
+	git status --porcelain --ignored >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! untracked-ignored/ignored
+!! untracked-ignored/test/ignored
+EOF
+
+test_expect_success 'status untracked directory with ignored files with --ignore -u' '
+	git status --porcelain --ignored -u >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+EOF
+
+test_expect_success 'status ignored tracked directory with --ignore' '
+	rm -rf untracked-ignored &&
+	mkdir tracked &&
+	: >tracked/committed &&
+	git add tracked/committed &&
+	git commit -m. &&
+	echo "tracked" >.gitignore &&
+	git status --porcelain --ignored >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+EOF
+
+test_expect_success 'status ignored tracked directory with --ignore -u' '
+	git status --porcelain --ignored -u >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! tracked/
+EOF
+
+test_expect_success 'status ignored tracked directory and uncommitted file with --ignore' '
+	: >tracked/uncommitted &&
+	git status --porcelain --ignored >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! tracked/uncommitted
+EOF
+
+test_expect_success 'status ignored tracked directory and uncommitted file with --ignore -u' '
+	git status --porcelain --ignored -u >actual &&
+	test_cmp expected actual
+'
+
+test_done
-- 
1.7.9.5

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

* Re: [PATCH 1/2] dir.c: Make git-status --ignored more consistent
  2012-12-30 14:39                     ` [PATCH 1/2] dir.c: Make git-status --ignored more consistent Antoine Pelisse
@ 2012-12-30 14:54                       ` Antoine Pelisse
  2012-12-30 15:01                         ` Adam Spiers
  2012-12-30 21:36                         ` Junio C Hamano
  0 siblings, 2 replies; 16+ messages in thread
From: Antoine Pelisse @ 2012-12-30 14:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Adam Spiers

By the way, that merges without conflicts with Adam's series, but it
will not compile as he renamed functions that I'm now using
(path_excluded() -> is_path_excluded() that is).

By the way, Junio, how do you handle this situation as a maintainer ?
Do you keep a note to manually make the change every time you remerge
the series together ? That is the kind of use-case you can't handle
with git-rerere, and I've been trying to find a solution to it.

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

* Re: [PATCH 1/2] dir.c: Make git-status --ignored more consistent
  2012-12-30 14:54                       ` Antoine Pelisse
@ 2012-12-30 15:01                         ` Adam Spiers
  2012-12-30 21:36                         ` Junio C Hamano
  1 sibling, 0 replies; 16+ messages in thread
From: Adam Spiers @ 2012-12-30 15:01 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: Junio C Hamano, git, Jeff King

On Sun, Dec 30, 2012 at 2:54 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
> By the way, that merges without conflicts with Adam's series, but it
> will not compile as he renamed functions that I'm now using
> (path_excluded() -> is_path_excluded() that is).

Ah, renames!  I forgot about those.

> By the way, Junio, how do you handle this situation as a maintainer ?
> Do you keep a note to manually make the change every time you remerge
> the series together ? That is the kind of use-case you can't handle
> with git-rerere, and I've been trying to find a solution to it.

Not sure if it helps to note that I am already basing my patch series
on top of Junio's nd/attr-match-optim-more branch.  Nguyen created
that branch which conflicted with mine, but then resolved the conflicts,
so I am basing mine on his to avoid having to continually resolve the
same conflicts.

So you could take the same approach and rebase yours on top of mine,
e.g.

    git remote add junio git://github.com/gitster/git.git
    git fetch junio
    git rebase junio/as/check-ignore

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

* Re: [PATCH 1/2] dir.c: Make git-status --ignored more consistent
  2012-12-30 14:54                       ` Antoine Pelisse
  2012-12-30 15:01                         ` Adam Spiers
@ 2012-12-30 21:36                         ` Junio C Hamano
  1 sibling, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2012-12-30 21:36 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: git, Jeff King, Adam Spiers

Antoine Pelisse <apelisse@gmail.com> writes:

> By the way, that merges without conflicts with Adam's series, but it
> will not compile as he renamed functions that I'm now using
> (path_excluded() -> is_path_excluded() that is).
>
> By the way, Junio, how do you handle this situation as a maintainer ?
> Do you keep a note to manually make the change every time you remerge
> the series together ? That is the kind of use-case you can't handle
> with git-rerere, and I've been trying to find a solution to it.

I'll finish the write-up on jc/doc-maintainer topic not in a very
distant future, but not today.

In the meantime, the hint is in the use of refs/merge-fix/ hierarchy
in the Reintegrate script found on my 'todo' branch (which I have a
separate clone/checkout of in "Meta/" directory in my main working
tree).

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

end of thread, other threads:[~2012-12-30 21:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-26  5:57 Bug in git status Michael Haggerty
2012-12-26 10:16 ` [PATCH] wt-status: Show ignored files in untracked dirs Antoine Pelisse
2012-12-26 13:31   ` [PATCH v2] " Antoine Pelisse
2012-12-27  2:37     ` Junio C Hamano
2012-12-27  3:48       ` Jeff King
2012-12-27  5:08         ` Junio C Hamano
2012-12-27 16:14           ` Antoine Pelisse
2012-12-27 16:19             ` Jeff King
2012-12-27 17:35               ` Antoine Pelisse
2012-12-28 14:05                 ` Antoine Pelisse
2012-12-29  7:22                   ` Jeff King
2012-12-30 14:39                     ` [PATCH 1/2] dir.c: Make git-status --ignored more consistent Antoine Pelisse
2012-12-30 14:54                       ` Antoine Pelisse
2012-12-30 15:01                         ` Adam Spiers
2012-12-30 21:36                         ` Junio C Hamano
2012-12-30 14:39                     ` [PATCH 2/2] git-status: Test --ignored behavior Antoine Pelisse

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).