Firstly, great work on the series! I've just started looking into it,
so please don't take my comments too seriously: some of them may be
queries, and others may be minor suggestions, but I can't say I
understand the area you're patching.  I know Junio doesn't like me
mixing queries in reviews, but I don't fully agree with his policy.

Karsten Blees wrote:
> 'git-status --ignored' drops ignored directories if they contain untracked
> files in an untracked sub directory.

Wait, ignored directories will always contain untracked
subdirectories, unless you add -f them, right?  Why are you saying
untracked files in an _untracked_ subdirectory?  We don't track
directories anyway, and I would call a directory "tracked" if there's
atleast one file inside it is tracked.  So, my understanding of this


In this example, if quux is ignored and untracked, git status
--ignored currently shows quux/.  If quux/bar is tracked (say with add
-f), but baz/ is untracked, git status --ignored doesn't show me
anything.  What exactly is the bug you're fixing?  I'll try to look at
the tests to infer this, but your commit message could probably be

Nit: please s/git-status/git status/

> Fix it by getting exact (recursive) excluded status in treat_directory.

Okay, so you're patching treat_directory() in dir.c to do some
recursive exclude handling.  Let's see what this is.

> diff --git a/dir.c b/dir.c
> index 57394e4..ec4eebf 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1060,6 +1060,15 @@ static enum directory_treatment treat_directory(struct 
> dir_struct *dir,
>         /* This is the "show_other_directories" case */
> +       /* might be a sub directory in an excluded directory */
> +       if (!exclude) {
> +               struct path_exclude_check check;
> +               int dt = DT_DIR;
> +               path_exclude_check_init(&check, dir);
> +               exclude = is_path_excluded(&check, dirname, len, &dt);
> +               path_exclude_check_clear(&check);
> +       }
> +

So, I'm guessing that DT_DIR refers to a value that a field in struct
dirent can take; that value could be one of DIR (directory), REG
(regular file?), LNK (symbolic link?).  I don't get much of this, but
what I do get is that you're setting exclude for the rest of the code
in this function.

Sorry that I'm not able to do a more thorough review.

> diff --git a/t/ b/t/
> index 0da1214..0f1034e 100755
> --- a/t/
> +++ b/t/
> @@ -143,4 +143,31 @@ test_expect_success 'status ignored tracked directory 
> and uncommitted file with
>         test_cmp expected actual
>  '
> +cat >expected <<\EOF
> +?? .gitignore
> +?? actual
> +?? expected
> +!! tracked/
> +EOF

Please put these segments inside the test_expect_success block, so
it's easy to think about those blocks in isolation.  I know you're
just following the existing conventions existing in this test, but
those are not necessarily good conventions.

> +test_expect_success 'status ignored tracked directory with uncommitted file 
> in untracked subdir with --ignore' '
> +       rm -rf tracked/uncommitted &&
> +       mkdir tracked/ignored &&
> +       : >tracked/ignored/uncommitted &&
> +       git status --porcelain --ignored >actual &&
> +       test_cmp expected actual
> +'

This is very confusing.  How is tracked a tracked directory?  Oh,
right: some previous test git add'ed tracked/committed.  How do I know
about that in this test?

Yeah, changes to tracked ignored directories are not shown, but the
commit message didn't tell me this.

> +cat >expected <<\EOF
> +?? .gitignore
> +?? actual
> +?? expected
> +!! tracked/ignored/uncommitted
> +EOF
> +
> +test_expect_success 'status ignored tracked directory with uncommitted file 
> in untracked subdir with --ignore -u' '
> +       git status --porcelain --ignored -u >actual &&
> +       test_cmp expected actual
> +'
> +
>  test_done

I suppose the commit message told me about this one vaguely, but I
think it could be much clearer overall.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to