Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
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 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
> 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 | |--+-+---| -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
Jeff King 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? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
On Wed, Dec 26, 2012 at 06:37:55PM -0800, Junio C Hamano wrote:
> Antoine Pelisse 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] wt-status: Show ignored files in untracked dirs
Antoine Pelisse 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
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] wt-status: Show ignored files in untracked dirs
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
