{"thread":{"id":"32456","subject":"Bug in git status","startedAt":"2012-12-26T05:57:03Z","lastAt":"2012-12-30T21:36:35Z","messageCount":16,"participants":["Michael Haggerty","Antoine Pelisse","Junio C Hamano","Jeff King","Adam Spiers"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"205487","messageId":"50DA91AF.1060200@alum.mit.edu","threadId":"32456","inReplyTo":null,"subject":"Bug in git status","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-12-26T05:57:03Z","receivedAt":"2012-12-26T05:57:03Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"I think I have found a bug in \"git status --untracked-files=all\n--ignored\", in both 1.8.0 and in master:\n\n$ git init status-test\nInitialized empty Git repository in\n/home/mhagger/self/proj/git/status-test/.git/\n$ cd status-test\n$ touch x\n$ touch x.ignore-me\n$ mkdir y\n$ touch y/foo\n$ touch y/foo.ignore-me\n$ git status --porcelain --untracked-files=all --ignored\n?? x\n?? x.ignore-me\n?? y/foo\n?? y/foo.ignore-me\n\nThe above output is what I expect. But if I add a .gitignore file, the\noutput of y/foo.ignore-me is incorrectly suppressed:\n\n$ echo '*.ignore-me' >.gitignore\n$ git status --porcelain --untracked-files=all --ignored\n?? .gitignore\n?? x\n?? y/foo\n!! x.ignore-me\n\nI came across this problem when trying to use the results of the above\ncommand to build a more flexible \"git clean\" type of script.\n\nI don't have time to look into this at the moment, if somebody wants to\njump in.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"205492","messageId":"1356516985-31068-1-git-send-email-apelisse@gmail.com","threadId":"32456","inReplyTo":"50DA91AF.1060200@alum.mit.edu","subject":"[PATCH] wt-status: Show ignored files in untracked dirs","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-26T10:16:25Z","receivedAt":"2012-12-26T10:16:25Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"When looking for ignored files, we do not recurse into untracked\ndirectory, and simply consider the directory ignored status.\nAs a consequence, we don't see ignored files in those directories.\n\nChange that behavior by recursing into untracked directories searching\nfor ignored files.\n\nSigned-off-by: Antoine Pelisse \n---\nI jumped in.\n\nThis seems to be broken since the creation of the --ignored option to\nwt-status.\n\nThis fixes the issue and breaks none of the existing tests.\nThe behavior seems sane to me, giving something like that:\n\n ?? .gitignore\n ?? x\n ?? y/foo\n !! x.ignore-me\n !! y/foo.ignore-me\n\nCheers,\nAntoine\n\n wt-status.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 2a9658b..7c41488 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -516,7 +516,7 @@ static void wt_status_collect_untracked(struct wt_status *s)\n\n \tif (s->show_ignored_files) {\n \t\tdir.nr = 0;\n-\t\tdir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;\n+\t\tdir.flags = DIR_SHOW_IGNORED;\n \t\tfill_directory(&dir, s->pathspec);\n \t\tfor (i = 0; i < dir.nr; i++) {\n \t\t\tstruct dir_entry *ent = dir.entries[i];\n--\n1.8.1.rc3.11.g86c3e6e.dirty\n"},{"id":"205494","messageId":"1356528674-2730-1-git-send-email-apelisse@gmail.com","threadId":"32456","inReplyTo":"1356516985-31068-1-git-send-email-apelisse@gmail.com","subject":"[PATCH v2] wt-status: Show ignored files in untracked dirs","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-26T13:31:14Z","receivedAt":"2012-12-26T13:31:14Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"When looking for ignored files, we do not recurse into untracked\ndirectory, and simply consider the directory ignored status.\nAs a consequence, we don't see ignored files in those directories.\n\nChange that behavior by recursing into untracked directories, if not\nignored themselves, searching for ignored files.\n\nSigned-off-by: Antoine Pelisse \n---\nActually, the previous patch breaks the case where the directory is ignored.\nThis one should fix both issues.\nLet me know if you see any other use case that could be an issue.\n\n dir.c | 7 +++++++\n wt-status.c | 2 +-\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 5a83aa7..2863799 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1042,6 +1042,13 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \t\t\treturn path_ignored;\n \t}\n\n+\t/*\n+\t * Don't recurse into ignored directories when looking for\n+\t * ignored files, but still show the directory as ignored.\n+\t */\n+\tif (exclude && (dir->flags & DIR_SHOW_IGNORED) && dtype == DT_DIR)\n+\t\treturn path_handled;\n+\n \tswitch (dtype) {\n \tdefault:\n \t\treturn path_ignored;\ndiff --git a/wt-status.c b/wt-status.c\nindex 2a9658b..7c41488 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -516,7 +516,7 @@ static void wt_status_collect_untracked(struct wt_status *s)\n\n \tif (s->show_ignored_files) {\n \t\tdir.nr = 0;\n-\t\tdir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;\n+\t\tdir.flags = DIR_SHOW_IGNORED;\n \t\tfill_directory(&dir, s->pathspec);\n \t\tfor (i = 0; i < dir.nr; i++) {\n \t\t\tstruct dir_entry *ent = dir.entries[i];\n--\n1.8.1.rc3.12.g8864e38\n"},{"id":"205546","messageId":"7vip7omd8c.fsf@alter.siamese.dyndns.org","threadId":"32456","inReplyTo":"1356528674-2730-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH v2] wt-status: Show ignored files in untracked dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-27T02:37:55Z","receivedAt":"2012-12-27T02:37:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse writes:\n\n> When looking for ignored files, we do not recurse into untracked\n> directory, and simply consider the directory ignored status.\n\nWhen asked to show ignored ones, instead of listing all ignored\nfiles in such a directory, we just say \"everything in this directory\nis ignored\"?\n\nThat sounds like a more desirable behaviour, than listing everything\nthere, at least to me, but perhaps I am missing something.\n\nCare to add a test for this new behaviour?\n\n> As a consequence, we don't see ignored files in those directories.\n>\n> Change that behavior by recursing into untracked directories, if not\n> ignored themselves, searching for ignored files.\n>\n> Signed-off-by: Antoine Pelisse \n> ---\n> Actually, the previous patch breaks the case where the directory is ignored.\n> This one should fix both issues.\n> Let me know if you see any other use case that could be an issue.\n>\n> dir.c | 7 +++++++\n> wt-status.c | 2 +-\n> 2 files changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 5a83aa7..2863799 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1042,6 +1042,13 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n> \t\t\treturn path_ignored;\n> \t}\n>\n> +\t/*\n> +\t * Don't recurse into ignored directories when looking for\n> +\t * ignored files, but still show the directory as ignored.\n> +\t */\n> +\tif (exclude && (dir->flags & DIR_SHOW_IGNORED) && dtype == DT_DIR)\n> +\t\treturn path_handled;\n> +\n> \tswitch (dtype) {\n> \tdefault:\n> \t\treturn path_ignored;\n> diff --git a/wt-status.c b/wt-status.c\n> index 2a9658b..7c41488 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -516,7 +516,7 @@ static void wt_status_collect_untracked(struct wt_status *s)\n>\n> \tif (s->show_ignored_files) {\n> \t\tdir.nr = 0;\n> -\t\tdir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;\n> +\t\tdir.flags = DIR_SHOW_IGNORED;\n> \t\tfill_directory(&dir, s->pathspec);\n> \t\tfor (i = 0; i < dir.nr; i++) {\n> \t\t\tstruct dir_entry *ent = dir.entries[i];\n> --\n> 1.8.1.rc3.12.g8864e38\n"},{"id":"205547","messageId":"20121227034859.GA20817@sigill.intra.peff.net","threadId":"32456","inReplyTo":"7vip7omd8c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] wt-status: Show ignored files in untracked dirs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-27T03:48:59Z","receivedAt":"2012-12-27T03:48:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 26, 2012 at 06:37:55PM -0800, Junio C Hamano wrote:\n\n> Antoine Pelisse writes:\n> \n> > When looking for ignored files, we do not recurse into untracked\n> > directory, and simply consider the directory ignored status.\n> \n> When asked to show ignored ones, instead of listing all ignored\n> files in such a directory, we just say \"everything in this directory\n> is ignored\"?\n> \n> That sounds like a more desirable behaviour, than listing everything\n> there, at least to me, but perhaps I am missing something.\n\nI do not use this feature myself, but I would think that it should\nrespect the same DIR_SHOW_OTHER_DIRECTORIES flag (or a parallel flag)\nthat we already hook into \"--untracked={all,normal}\".\n\nIOW, given:\n\n git init\n mkdir untracked ignored\n >untracked/file\n >ignored/file\n echo ignored >.git/info/exclude\n\nI would expect:\n\n $ git status --short --ignored --untracked=normal\n ?? untracked/\n !! ignored/\n\n $ git status --short --ignored --untracked=all\n ?? untracked/file\n !! ignored/file\n\nI do not know if anybody cares about the distinction, but optionally we\ncould give --ignored its own selector, like:\n\n $ git status --short --ignored=all --untracked=normal\n ?? untracked/\n !! ignored/file\n\nwhere obviously it would default to \"none\" (whereas untracked defaults\nto \"normal\"). But the behavior with Antoine's patch is:\n\n $ git status --short --ignored --untracked=normal\n ?? untracked/\n !! ignored\n\n $ git status --short --ignored --untracked=all\n ?? untracked/file\n !! ignored\n\nwhich seems wrong to me for two reasons:\n\n 1. It does not recurse for ignored but untracked entries. Neither does\n the current code, but I think it should.\n\n 2. It loses the trailing slash from the ignored directory in both\n cases (which is printed by the current code).\n\n-Peff\n"},{"id":"205548","messageId":"7va9t0m69o.fsf@alter.siamese.dyndns.org","threadId":"32456","inReplyTo":"20121227034859.GA20817@sigill.intra.peff.net","subject":"Re: [PATCH v2] wt-status: Show ignored files in untracked dirs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-27T05:08:19Z","receivedAt":"2012-12-27T05:08:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> IOW, given:\n>\n> git init\n> mkdir untracked ignored\n> >untracked/file\n> >ignored/file\n> echo ignored >.git/info/exclude\n>\n> I would expect:\n>\n> $ git status --short --ignored --untracked=normal\n> ?? untracked/\n> !! ignored/\n\nSensible.\n\n> $ git status --short --ignored --untracked=all\n> ?? untracked/file\n> !! ignored/file\n\nAgain sensible; OK, --untracked=all is what I was missing.\n\n> I do not know if anybody cares about the distinction, but optionally we\n> could give --ignored its own selector, like:\n>\n> $ git status --short --ignored=all --untracked=normal\n> ?? untracked/\n> !! ignored/file\n>\n> where obviously it would default to \"none\" (whereas untracked defaults\n> to \"normal\").\n\nWe could just say the selector for the ignored implicitly follows\nwhat is given for --untracked, if we don't care.\n\n> But the behavior with Antoine's patch is:\n>\n> $ git status --short --ignored --untracked=normal\n> ?? untracked/\n> !! ignored\n>\n> $ git status --short --ignored --untracked=all\n> ?? untracked/file\n> !! ignored\n>\n> which seems wrong to me for two reasons:\n>\n> 1. It does not recurse for ignored but untracked entries. Neither does\n> the current code, but I think it should.\n>\n> 2. It loses the trailing slash from the ignored directory in both\n> cases (which is printed by the current code).\n\nNicely analysed. Perhaps we would want new test pieces to define\nthe behaviour we want to see first?\n"},{"id":"205555","messageId":"CALWbr2wFg_9oDoZ_BUQwAzVV+UJSqBQRrMYmt6fv=fo02RL7Zg@mail.gmail.com","threadId":"32456","inReplyTo":"7va9t0m69o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] wt-status: Show ignored files in untracked dirs","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-27T16:14:54Z","receivedAt":"2012-12-27T16:14:54Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"> Nicely analysed. Perhaps we would want new test pieces to define\n> the behaviour we want to see first?\n\nI think we should.\n\nI also thought about the use case of \"committed\" and ignored directory\nwhich is also broken to me (point 3 in the table below).\n\nAnyway I tried to make a table to sum-up/discuss the list of behaviors\nwe would like to see/test, taking Jeff mail into account.\n(warning: that requires fixed width font)\n\n|----------------------+---------------------+---------------------------|\n| Output | A. status --ignored | B. status --ignored -uall |\n| | | (or with potential |\n| | | --ignored=all) |\n|----------------------+---------------------+---------------------------|\n| 1. Untracked dirU | Current: | Current: |\n| with ignored unco.ig | Empty | Empty |\n| in it | | |\n| | Expected: | Expected: |\n| | !!dirU/ | !!dirU/unco.ig |\n|----------------------+---------------------+---------------------------|\n| 2. Untracked and | Current (OK): | Current: |\n| ignored dirU with | !!dirU/ | !!dirU/ |\n| file in it | | |\n| | | Expected: |\n| | | !!dirU/unco |\n|----------------------+---------------------+---------------------------|\n| 3. \"Committed\" dirA | Current: | Current: |\n| yet ignored | Empty | Empty |\n| with uncommitted | | |\n| file in it | Expected: | Expected: |\n| | dirA/ | dirA/unco |\n|----------------------+---------------------+---------------------------|\n"},{"id":"205554","messageId":"20121227161920.GA28162@sigill.intra.peff.net","threadId":"32456","inReplyTo":"CALWbr2wFg_9oDoZ_BUQwAzVV+UJSqBQRrMYmt6fv=fo02RL7Zg@mail.gmail.com","subject":"Re: [PATCH v2] wt-status: Show ignored files in untracked dirs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-27T16:19:20Z","receivedAt":"2012-12-27T16:19:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 27, 2012 at 05:14:54PM +0100, Antoine Pelisse wrote:\n\n> > Nicely analysed. Perhaps we would want new test pieces to define\n> > the behaviour we want to see first?\n> \n> I think we should.\n> \n> I also thought about the use case of \"committed\" and ignored directory\n> which is also broken to me (point 3 in the table below).\n\nBy \"committed\", I assume you meat that you have \"dirA/unco\" as an\nuntracked file, and \"dirA/committed\" as a file in the index?\n\n> Anyway I tried to make a table to sum-up/discuss the list of behaviors\n> we would like to see/test, taking Jeff mail into account.\n> (warning: that requires fixed width font)\n> \n> |----------------------+---------------------+---------------------------|\n> | Output | A. status --ignored | B. status --ignored -uall |\n> | | | (or with potential |\n> | | | --ignored=all) |\n> |----------------------+---------------------+---------------------------|\n> | 1. Untracked dirU | Current: | Current: |\n> | with ignored unco.ig | Empty | Empty |\n> | in it | | |\n> | | Expected: | Expected: |\n> | | !!dirU/ | !!dirU/unco.ig |\n> |----------------------+---------------------+---------------------------|\n> | 2. Untracked and | Current (OK): | Current: |\n> | ignored dirU with | !!dirU/ | !!dirU/ |\n> | file in it | | |\n> | | | Expected: |\n> | | | !!dirU/unco |\n> |----------------------+---------------------+---------------------------|\n> | 3. \"Committed\" dirA | Current: | Current: |\n> | yet ignored | Empty | Empty |\n> | with uncommitted | | |\n> | file in it | Expected: | Expected: |\n> | | dirA/ | dirA/unco |\n> |----------------------+---------------------+---------------------------|\n\nThanks for putting this together. I agree with the expected output in\neach case, and I think this covers the cases we have seen (case 1 is\nMichael's original report, case 2 is what I wrote in my mail, and case 3\nis the one you just came up with). I can't think offhand of any others.\n\n-Peff\n"},{"id":"205557","messageId":"CALWbr2xmtvchR4G37-FzzgScKe4p4RjLc7=Pg8d4K6SWO7tGAQ@mail.gmail.com","threadId":"32456","inReplyTo":"20121227161920.GA28162@sigill.intra.peff.net","subject":"Re: [PATCH v2] wt-status: Show ignored files in untracked dirs","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-27T17:35:06Z","receivedAt":"2012-12-27T17:35:06Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"> By \"committed\", I assume you meat that you have \"dirA/unco\" as an\n> untracked file, and \"dirA/committed\" as a file in the index?\n\nOf course,\n\n> Thanks for putting this together. I agree with the expected output in\n> each case, and I think this covers the cases we have seen (case 1 is\n> Michael's original report, case 2 is what I wrote in my mail, and case 3\n> is the one you just came up with). I can't think offhand of any others.\n\nGreat, so I can build some tests reflecting those behaviors while\nwaiting more inputs\n"},{"id":"205588","messageId":"CALWbr2yNR=K3MqBVe=sfSxPaJ+-A8utHBqoiHPHPLxr_9e9SVQ@mail.gmail.com","threadId":"32456","inReplyTo":"CALWbr2xmtvchR4G37-FzzgScKe4p4RjLc7=Pg8d4K6SWO7tGAQ@mail.gmail.com","subject":"Re: [PATCH v2] wt-status: Show ignored files in untracked dirs","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-28T14:05:30Z","receivedAt":"2012-12-28T14:05:30Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Hey Peff,\nI actually have an issue with the behavior we discussed (referenced as 1.A.)\n\nUsing the example from Michael's mail, I end up having this:\n$ git status --porcelain --ignored\n?? .gitignore\n?? x\n?? y/\n!! x.ignore-me\n!! y/\n\ny/ is referred as untracked, because it contains untracked files, and\nthen as ignored because it\ncontains ignored files.\n\nShowing it twice doesn't feel right though, so I guess we should only\nshow \"?? y/\" with untracked=normal,\nand \"!! y/foo.ignore-me\" when using untracked=all\n\nWhat do you think ?\n\n\n\n\nOn Thu, Dec 27, 2012 at 6:35 PM, Antoine Pelisse wrote:\n>> By \"committed\", I assume you meat that you have \"dirA/unco\" as an\n>> untracked file, and \"dirA/committed\" as a file in the index?\n>\n> Of course,\n>\n>> Thanks for putting this together. I agree with the expected output in\n>> each case, and I think this covers the cases we have seen (case 1 is\n>> Michael's original report, case 2 is what I wrote in my mail, and case 3\n>> is the one you just came up with). I can't think offhand of any others.\n>\n> Great, so I can build some tests reflecting those behaviors while\n> waiting more inputs\n"},{"id":"205640","messageId":"20121229072249.GB15408@sigill.intra.peff.net","threadId":"32456","inReplyTo":"CALWbr2yNR=K3MqBVe=sfSxPaJ+-A8utHBqoiHPHPLxr_9e9SVQ@mail.gmail.com","subject":"Re: [PATCH v2] wt-status: Show ignored files in untracked dirs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-12-29T07:22:49Z","receivedAt":"2012-12-29T07:22:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 28, 2012 at 03:05:30PM +0100, Antoine Pelisse wrote:\n\n> Using the example from Michael's mail, I end up having this:\n> $ git status --porcelain --ignored\n> ?? .gitignore\n> ?? x\n> ?? y/\n> !! x.ignore-me\n> !! y/\n> \n> y/ is referred as untracked, because it contains untracked files, and\n> then as ignored because it\n> contains ignored files.\n> \n> Showing it twice doesn't feel right though, so I guess we should only\n> show \"?? y/\" with untracked=normal,\n> and \"!! y/foo.ignore-me\" when using untracked=all\n> \n> What do you think ?\n\nGood catch. I agree that showing just \"?? y/\" in the untracked=normal\ncase makes sense. It makes the definition of \"!!\" to mean \"all untracked\nfiles in this path are ignored\". IOW, showing \"??\" means there are one\nor more untracked, unignored files. There may _also_ be ignored files,\nbut we do not say (nor we even necessarily need to bother checking).\n\nIn retrospect, I think it might have made more sense to use the second\ncharacter of an untracked line to represent \"ignored\". That is, the\noutput:\n\n ?? .gitignore\n ?? x\n ?! y/\n !! x.ignore-me\n\nwould make sense to me. But that would be a backwards-incompatible\nchange at this point, and I don't think it's worth it.\n\n-Peff\n"},{"id":"205674","messageId":"1356878341-12942-1-git-send-email-apelisse@gmail.com","threadId":"32456","inReplyTo":"20121229072249.GB15408@sigill.intra.peff.net","subject":"[PATCH 1/2] dir.c: Make git-status --ignored more consistent","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-30T14:39:00Z","receivedAt":"2012-12-30T14:39:00Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"The current behavior of git-status is inconsistent and\nmisleading. Especially when used with --untracked-files=all option:\n\n - files ignored in untracked directories will be missing from status\n output.\n - untracked files in committed yet ignored directories are also\n missing.\n - with --untracked-files=normal, untracked directories that contains\n only ignored files are dropped too.\n\nMake the behavior more consistent across all possible use cases:\n\n - \"--ignored --untracked-files=normal\" doesn't show each specific\n files but top directory.\n Shows untracked directories that only contains ignored files, and\n ignored tracked directories with untracked files.\n - \"--ignored --untracked-files=all\" shows all ignored files, either\n because it's in an ignored directory (tracked or untracked), or\n because the file is explicitly ignored.\n\nSigned-off-by: Antoine Pelisse \n---\n dir.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++------------\n wt-status.c | 4 ++-\n 2 files changed, 81 insertions(+), 21 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 5a83aa7..d0c92dc 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -834,8 +834,9 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len)\n * traversal routine.\n *\n * Case 1: If we *already* have entries in the index under that\n- * directory name, we always recurse into the directory to see\n- * all the files.\n+ * directory name, we recurse into the directory to see all the files,\n+ * unless the directory is excluded and we want to show ignored\n+ * directories\n *\n * Case 2: If we *already* have that directory name as a gitlink,\n * we always continue to see it as a gitlink, regardless of whether\n@@ -849,6 +850,9 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len)\n * just a directory, unless \"hide_empty_directories\" is\n * also true and the directory is empty, in which case\n * we just ignore it entirely.\n+ * if we are looking for ignored directories, look if it\n+ * contains only ignored files to decide if it must be shown as\n+ * ignored or not.\n * (b) if it looks like a git directory, and we don't have\n * 'no_gitlinks' set we treat it as a gitlink, and show it\n * as a directory.\n@@ -861,12 +865,15 @@ enum directory_treatment {\n };\n \n static enum directory_treatment treat_directory(struct dir_struct *dir,\n-\tconst char *dirname, int len,\n+\tconst char *dirname, int len, int exclude,\n \tconst struct path_simplify *simplify)\n {\n \t/* The \"len-1\" is to strip the final '/' */\n \tswitch (directory_exists_in_index(dirname, len-1)) {\n \tcase index_directory:\n+\t\tif ((dir->flags & DIR_SHOW_OTHER_DIRECTORIES) && exclude)\n+\t\t\tbreak;\n+\n \t\treturn recurse_into_directory;\n \n \tcase index_gitdir:\n@@ -886,7 +893,23 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,\n \t}\n \n \t/* This is the \"show_other_directories\" case */\n-\tif (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))\n+\n+\t/*\n+\t * We are looking for ignored files and our directory is not ignored,\n+\t * check if it contains only ignored files\n+\t */\n+\tif ((dir->flags & DIR_SHOW_IGNORED) && !exclude) {\n+\t\tint ignored;\n+\t\tdir->flags &= ~DIR_SHOW_IGNORED;\n+\t\tdir->flags |= DIR_HIDE_EMPTY_DIRECTORIES;\n+\t\tignored = read_directory_recursive(dir, dirname, len, 1, simplify);\n+\t\tdir->flags &= ~DIR_HIDE_EMPTY_DIRECTORIES;\n+\t\tdir->flags |= DIR_SHOW_IGNORED;\n+\n+\t\treturn ignored ? ignore_directory : show_directory;\n+\t}\n+\tif (!(dir->flags & DIR_SHOW_IGNORED) &&\n+\t !(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))\n \t\treturn show_directory;\n \tif (!read_directory_recursive(dir, dirname, len, 1, simplify))\n \t\treturn ignore_directory;\n@@ -894,6 +917,49 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,\n }\n \n /*\n+ * Decide what to do when we find a file while traversing the\n+ * filesystem. Mostly two cases:\n+ *\n+ * 1. We are looking for ignored files\n+ * (a) File is ignored, include it\n+ * (b) File is in ignored path, include it\n+ * (c) File is not ignored, exclude it\n+ *\n+ * 2. Other scenarios, include the file if not excluded\n+ *\n+ * Return 1 for exclude, 0 for include.\n+ */\n+static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude, int *dtype)\n+{\n+\tstruct path_exclude_check check;\n+\tint exclude_file = 0;\n+\n+\tif (exclude)\n+\t\texclude_file = !(dir->flags & DIR_SHOW_IGNORED);\n+\telse if (dir->flags & DIR_SHOW_IGNORED) {\n+\t\t/*\n+\t\t * Optimization:\n+\t\t * Don't spend time on indexed files, they won't be\n+\t\t * added to the list anyway\n+\t\t */\n+\t\tstruct cache_entry *ce = index_name_exists(&the_index,\n+\t\t path->buf, path->len, ignore_case);\n+\n+\t\tif (ce)\n+\t\t\treturn 1;\n+\n+\t\tpath_exclude_check_init(&check, dir);\n+\n+\t\tif (!path_excluded(&check, path->buf, path->len, dtype))\n+\t\t\texclude_file = 1;\n+\n+\t\tpath_exclude_check_clear(&check);\n+\t}\n+\n+\treturn exclude_file;\n+}\n+\n+/*\n * This is an inexact early pruning of any recursive directory\n * reading - if the path cannot possibly be in the pathspec,\n * return true, and we'll skip it early.\n@@ -1031,27 +1097,14 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \tif (dtype == DT_UNKNOWN)\n \t\tdtype = get_dtype(de, path->buf, path->len);\n \n-\t/*\n-\t * Do we want to see just the ignored files?\n-\t * We still need to recurse into directories,\n-\t * even if we don't ignore them, since the\n-\t * directory may contain files that we do..\n-\t */\n-\tif (!exclude && (dir->flags & DIR_SHOW_IGNORED)) {\n-\t\tif (dtype != DT_DIR)\n-\t\t\treturn path_ignored;\n-\t}\n-\n \tswitch (dtype) {\n \tdefault:\n \t\treturn path_ignored;\n \tcase DT_DIR:\n \t\tstrbuf_addch(path, '/');\n-\t\tswitch (treat_directory(dir, path->buf, path->len, simplify)) {\n+\n+\t\tswitch (treat_directory(dir, path->buf, path->len, exclude, simplify)) {\n \t\tcase show_directory:\n-\t\t\tif (exclude != !!(dir->flags\n-\t\t\t\t\t & DIR_SHOW_IGNORED))\n-\t\t\t\treturn path_ignored;\n \t\t\tbreak;\n \t\tcase recurse_into_directory:\n \t\t\treturn path_recurse;\n@@ -1061,7 +1114,12 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \t\tbreak;\n \tcase DT_REG:\n \tcase DT_LNK:\n-\t\tbreak;\n+\t\tswitch(treat_file(dir, path, exclude, &dtype)) {\n+\t\tcase 1:\n+\t\t\treturn path_ignored;\n+\t\tdefault:\n+\t\t\tbreak;\n+\t\t}\n \t}\n \treturn path_handled;\n }\ndiff --git a/wt-status.c b/wt-status.c\nindex 2a9658b..d7cfe8f 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -516,7 +516,9 @@ static void wt_status_collect_untracked(struct wt_status *s)\n \n \tif (s->show_ignored_files) {\n \t\tdir.nr = 0;\n-\t\tdir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;\n+\t\tdir.flags = DIR_SHOW_IGNORED;\n+\t\tif (s->show_untracked_files != SHOW_ALL_UNTRACKED_FILES)\n+\t\t\tdir.flags |= DIR_SHOW_OTHER_DIRECTORIES;\n \t\tfill_directory(&dir, s->pathspec);\n \t\tfor (i = 0; i < dir.nr; i++) {\n \t\t\tstruct dir_entry *ent = dir.entries[i];\n-- \n1.7.9.5\n"},{"id":"205675","messageId":"1356878341-12942-2-git-send-email-apelisse@gmail.com","threadId":"32456","inReplyTo":"1356878341-12942-1-git-send-email-apelisse@gmail.com","subject":"[PATCH 2/2] git-status: Test --ignored behavior","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-30T14:39:01Z","receivedAt":"2012-12-30T14:39:01Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Test all possible use-cases of git-status --ignored with\n--untracked-files to normal and all:\n\n - untracked directory is listed as untracked if it has a mix of\n untracked and ignored files in it.\n with -uall, ignored/untracked files are listed as\n ignored/untracked.\n\n - untracked directory with only ignored files is listed as ignored.\n with -uall, all files in the directory are listed.\n\n - ignored directory is listed as ignored. With -uall, all files in\n the directory are listed as ignored.\n\n - ignored and committed directory is listed as ignored if it has\n untracked files.\n with -uall, all untracked files in the directory are listed as\n ignored.\n\nSigned-off-by: Antoine Pelisse \n---\n t/t7061-wtstatus-ignore.sh | 146 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 146 insertions(+)\n create mode 100755 t/t7061-wtstatus-ignore.sh\n\ndiff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh\nnew file mode 100755\nindex 0000000..0da1214\n--- /dev/null\n+++ b/t/t7061-wtstatus-ignore.sh\n@@ -0,0 +1,146 @@\n+#!/bin/sh\n+\n+test_description='git-status ignored files'\n+\n+. ./test-lib.sh\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+?? untracked/\n+EOF\n+\n+test_expect_success 'status untracked directory with --ignored' '\n+\techo \"ignored\" >.gitignore &&\n+\tmkdir untracked &&\n+\t: >untracked/ignored &&\n+\t: >untracked/uncommitted &&\n+\tgit status --porcelain --ignored >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+?? untracked/uncommitted\n+!! untracked/ignored\n+EOF\n+\n+test_expect_success 'status untracked directory with --ignored -u' '\n+\tgit status --porcelain --ignored -u >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+!! ignored/\n+EOF\n+\n+test_expect_success 'status ignored directory with --ignore' '\n+\trm -rf untracked &&\n+\tmkdir ignored &&\n+\t: >ignored/uncommitted &&\n+\tgit status --porcelain --ignored >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+!! ignored/uncommitted\n+EOF\n+\n+test_expect_success 'status ignored directory with --ignore -u' '\n+\tgit status --porcelain --ignored -u >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+!! untracked-ignored/\n+EOF\n+\n+test_expect_success 'status untracked directory with ignored files with --ignore' '\n+\trm -rf ignored &&\n+\tmkdir untracked-ignored &&\n+\tmkdir untracked-ignored/test &&\n+\t: >untracked-ignored/ignored &&\n+\t: >untracked-ignored/test/ignored &&\n+\tgit status --porcelain --ignored >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+!! untracked-ignored/ignored\n+!! untracked-ignored/test/ignored\n+EOF\n+\n+test_expect_success 'status untracked directory with ignored files with --ignore -u' '\n+\tgit status --porcelain --ignored -u >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+EOF\n+\n+test_expect_success 'status ignored tracked directory with --ignore' '\n+\trm -rf untracked-ignored &&\n+\tmkdir tracked &&\n+\t: >tracked/committed &&\n+\tgit add tracked/committed &&\n+\tgit commit -m. &&\n+\techo \"tracked\" >.gitignore &&\n+\tgit status --porcelain --ignored >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+EOF\n+\n+test_expect_success 'status ignored tracked directory with --ignore -u' '\n+\tgit status --porcelain --ignored -u >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+!! tracked/\n+EOF\n+\n+test_expect_success 'status ignored tracked directory and uncommitted file with --ignore' '\n+\t: >tracked/uncommitted &&\n+\tgit status --porcelain --ignored >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected <<\\EOF\n+?? .gitignore\n+?? actual\n+?? expected\n+!! tracked/uncommitted\n+EOF\n+\n+test_expect_success 'status ignored tracked directory and uncommitted file with --ignore -u' '\n+\tgit status --porcelain --ignored -u >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n1.7.9.5\n"},{"id":"205676","messageId":"CALWbr2w=CWkpbJhC5sjd9HnErmWj9JQnD6UUiDM91ovJ_-16vA@mail.gmail.com","threadId":"32456","inReplyTo":"1356878341-12942-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH 1/2] dir.c: Make git-status --ignored more consistent","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-30T14:54:14Z","receivedAt":"2012-12-30T14:54:14Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"By the way, that merges without conflicts with Adam's series, but it\nwill not compile as he renamed functions that I'm now using\n(path_excluded() -> is_path_excluded() that is).\n\nBy the way, Junio, how do you handle this situation as a maintainer ?\nDo you keep a note to manually make the change every time you remerge\nthe series together ? That is the kind of use-case you can't handle\nwith git-rerere, and I've been trying to find a solution to it.\n"},{"id":"205678","messageId":"CAOkDyE_N-fOMsrSaHKyu=M1PAN5h0JAYJ9ekN=q7skg9SOzSKg@mail.gmail.com","threadId":"32456","inReplyTo":"CALWbr2w=CWkpbJhC5sjd9HnErmWj9JQnD6UUiDM91ovJ_-16vA@mail.gmail.com","subject":"Re: [PATCH 1/2] dir.c: Make git-status --ignored more consistent","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-30T15:01:22Z","receivedAt":"2012-12-30T15:01:22Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Sun, Dec 30, 2012 at 2:54 PM, Antoine Pelisse wrote:\n> By the way, that merges without conflicts with Adam's series, but it\n> will not compile as he renamed functions that I'm now using\n> (path_excluded() -> is_path_excluded() that is).\n\nAh, renames! I forgot about those.\n\n> By the way, Junio, how do you handle this situation as a maintainer ?\n> Do you keep a note to manually make the change every time you remerge\n> the series together ? That is the kind of use-case you can't handle\n> with git-rerere, and I've been trying to find a solution to it.\n\nNot sure if it helps to note that I am already basing my patch series\non top of Junio's nd/attr-match-optim-more branch. Nguyen created\nthat branch which conflicted with mine, but then resolved the conflicts,\nso I am basing mine on his to avoid having to continually resolve the\nsame conflicts.\n\nSo you could take the same approach and rebase yours on top of mine,\ne.g.\n\n git remote add junio git://github.com/gitster/git.git\n git fetch junio\n git rebase junio/as/check-ignore\n"},{"id":"205686","messageId":"7v623jfcik.fsf@alter.siamese.dyndns.org","threadId":"32456","inReplyTo":"CALWbr2w=CWkpbJhC5sjd9HnErmWj9JQnD6UUiDM91ovJ_-16vA@mail.gmail.com","subject":"Re: [PATCH 1/2] dir.c: Make git-status --ignored more consistent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-30T21:36:35Z","receivedAt":"2012-12-30T21:36:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse writes:\n\n> By the way, that merges without conflicts with Adam's series, but it\n> will not compile as he renamed functions that I'm now using\n> (path_excluded() -> is_path_excluded() that is).\n>\n> By the way, Junio, how do you handle this situation as a maintainer ?\n> Do you keep a note to manually make the change every time you remerge\n> the series together ? That is the kind of use-case you can't handle\n> with git-rerere, and I've been trying to find a solution to it.\n\nI'll finish the write-up on jc/doc-maintainer topic not in a very\ndistant future, but not today.\n\nIn the meantime, the hint is in the use of refs/merge-fix/ hierarchy\nin the Reintegrate script found on my 'todo' branch (which I have a\nseparate clone/checkout of in \"Meta/\" directory in my main working\ntree).\n"}]}