git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 1/4] status: add option to show ignored files differently

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 23, 2017, 03:34 UTC
Message-ID
<xmqqzi8ibkq6.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20171019160601.9382-2-jamill@microsoft.com>
Jameson Miller <jameson.miller81@gmail.com> writes:
Show 19 quoted lines
> Teach the status command more flexibility in how ignored files are
> reported. Currently, the reporting of ignored files and untracked
> files are linked. You cannot control how ignored files are reported
> independently of how untracked files are reported (i.e. `all` vs
> `normal`). This makes it impossible to show untracked files with the
> `all` option, but show ignored files with the `normal` option.
> 
> This work 1) adds the ability to control the reporting of ignored
> files independently of untracked files and 2) introduces the concept
> of status reporting ignored paths that explicitly match an ignored
> pattern. ...
>
> When status is set to report matching ignored files, it has the
> following behavior. Ignored files and directories that explicitly
> match an exclude pattern are reported. If an ignored directory matches
> an exclude pattern, then the path of the directory is returned. If a
> directory does not match an exclude pattern, but all of its contents
> are ignored, then the contained files are reported instead of the
> directory.
Thanks for an updated log message.  Very nicely explained.
Show 26 quoted lines
> +static void handle_ignored_arg(struct wt_status *s)
> +{
> +	if (!ignored_arg)
> +		; /* default already initialized */
> +	else if (!strcmp(ignored_arg, "traditional"))
> +		s->show_ignored_mode = SHOW_TRADITIONAL_IGNORED;
> +	else if (!strcmp(ignored_arg, "no"))
> +		s->show_ignored_mode = SHOW_NO_IGNORED;
> +	else if (!strcmp(ignored_arg, "matching"))
> +		s->show_ignored_mode = SHOW_MATCHING_IGNORED;
> +	else
> +		die(_("Invalid ignored mode '%s'"), ignored_arg);
> +}
>  
>  static void handle_untracked_files_arg(struct wt_status *s)
>  {
> @@ -1363,8 +1376,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>  		  N_("mode"),
>  		  N_("show untracked files, optional modes: all, normal, no. (Default: all)"),
>  		  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
> -		OPT_BOOL(0, "ignored", &show_ignored_in_status,
> -			 N_("show ignored files")),
> +		{ OPTION_STRING, 0, "ignored", &ignored_arg,
> +		  N_("mode"),
> +		  N_("show ignored files, optional modes: traditional, matching, no. (Default: traditional)"),
> +		  PARSE_OPT_OPTARG, NULL, (intptr_t)"traditional" },

We used to use "show_ignored_in_status" variable that can be either 0 (when no --ignored is given, or an explicit --no-ignored is given, on the command line) or 1 (when --ignored is given on the command line).

We still allow "--ignored" without value for backward compatibility, but 

    $ git status -uall --ignored \*.c

may trigger die() from handle_ignored_arg(). I wonder if this is something we care about.

	... goes and digs ...

OK, because of PARSE_OPT_OPTARG, the above is safe; "*.c" is not mistaken as the value given to the --ignored option. It also means that

    $ git status -uno --ignored matching

would not mean what a naïve reader may expect and does not error out, even though

    $ git status -uno --ignored=matching

would. Which is something we eventually might care about, but that is how parse-options PARSE_OPT_OPTARG works, and I consider "fixing" it is totally out of the scope of this series (e.g. the next option "--ignore-submodules" below shares exactly the same issue).

>  		{ OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg, N_("when"),
>  		  N_("ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)"),
>  		  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
Show 10 quoted lines
> diff --git a/wt-status.c b/wt-status.c
> index 6f730ee8f2..8301c84946 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -660,10 +660,15 @@ static void wt_status_collect_untracked(struct wt_status *s)
>  	if (s->show_untracked_files != SHOW_ALL_UNTRACKED_FILES)
>  		dir.flags |=
>  			DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
> -	if (s->show_ignored_files)
> +	if (s->show_ignored_mode) {

Now we no longer use show_ignored_files that was a boolean yes/no, and instead use an enum show_ignored_mode, we'd better spell this out like so:

	if (s->show_ignored_mode == SHOW_NO_IGNORED) {
Show 5 quoted lines
>  		dir.flags |= DIR_SHOW_IGNORED_TOO;
> -	else
> +
> +		if (s->show_ignored_mode == SHOW_MATCHING_IGNORED)
> +			dir.flags |= DIR_SHOW_IGNORED_TOO_MODE_MATCHING;

Mental note: the old "show_ignored_files is true" case is now split into two, i.e. traditional vs matching. When matching is used, dir.flags gets a new bit, i.e. DIR_SHOW_IGNORED_TOO_MODE_MATCHING. Lack of that bit means dir.c helpers should behave as before.

Show 13 quoted lines
> +	} else {
>  		dir.untracked = the_index.untracked;
> +	}
> +
>  	setup_standard_excludes(&dir);
>  
>  	fill_directory(&dir, &the_index, &s->pathspec);
> @@ -1621,7 +1626,7 @@ static void wt_longstatus_print(struct wt_status *s)
>  	}
>  	if (s->show_untracked_files) {
>  		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
> -		if (s->show_ignored_files)
> +		if (s->show_ignored_mode)
Likewise, i.e.
	if (s->show_ignored_mode == SHOW_NO_IGNORED)
>  			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
>  		if (advice_status_u_option && 2000 < s->untracked_in_ms) {
>  			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
Show 11 quoted lines
> diff --git a/dir.c b/dir.c
> index 1d17b800cf..b9af87eca9 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1389,6 +1389,30 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
>  	case index_nonexistent:
>  		if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES)
>  			break;
> +		if (exclude &&
> +			(dir->flags & DIR_SHOW_IGNORED_TOO) &&
> +			(dir->flags & DIR_SHOW_IGNORED_TOO_MODE_MATCHING)) {

It somehow feels superfluous to check both TOO and TOO_MODE_MATCHING here (IOW, wouldn't the latter be superset of the former? Is there a valid case where the latter is set but not the former?), but it is perhaps OK as belt-and-suspenders sanity.

Show 11 quoted lines
> +			/*
> +			 * This is an excluded directory and we are
> +			 * showing ignored paths that match an exclude
> +			 * pattern.  (e.g. show directory as ignored
> +			 * only if it matches an exclude pattern).
> +			 * This path will either be 'path_excluded`
> +			 * (if we are showing empty directories or if
> +			 * the directory is not empty), or will be
> +			 * 'path_none' (empty directory, and we are
> +			 * not showing empty directories).
> +			 */

The only caller of treat_directory() calls us when the path we are seeing (sans trailing '/' that was added immediately before we were called) passed "is_excluded()" check in the caller, so at this point, the directory in question is being excluded because it matched an exclude pattern. IOW, if we are a/b/c, a pattern such as "a/b/*" may have said we are excluded and made the call to this function. Do we need to ensure that a pattern like "a/*" is not what excluded us? Or IGNORED_TOO_MODE_MATCHING also should show "a/b/c" when a pattern "a/*" matched it as "ignored because it matched an exclude pattern"?

Show 9 quoted lines
> +			if (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))
> +				return path_excluded;
> +
> +			if (read_directory_recursive(dir, istate, dirname, len,
> +						     untracked, 1, 1, pathspec) == path_excluded)
> +				return path_excluded;
> +
> +			return path_none;
> +		}

... I guess we won't get into such a situation because this codepath will return only _excluded or _none when we are handing "a/b" which is already excluded "a/*", and does not cause the caller to recurse into "a/b/c" in the first place. So the updated code looks fine.

Show 17 quoted lines
>  		if (!(dir->flags & DIR_NO_GITLINKS)) {
>  			unsigned char sha1[20];
>  			if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0)
> diff --git a/dir.h b/dir.h
> index e3717055d1..57b0943dae 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -152,7 +152,8 @@ struct dir_struct {
>  		DIR_COLLECT_IGNORED = 1<<4,
>  		DIR_SHOW_IGNORED_TOO = 1<<5,
>  		DIR_COLLECT_KILLED_ONLY = 1<<6,
> -		DIR_KEEP_UNTRACKED_CONTENTS = 1<<7
> +		DIR_KEEP_UNTRACKED_CONTENTS = 1<<7,
> +		DIR_SHOW_IGNORED_TOO_MODE_MATCHING = 1<<8
>  	} flags;
>  	struct dir_entry **entries;
>  	struct dir_entry **ignored;
Previous: Jameson MillerNext: Jameson Miller
Message 32 of 49 in “Teach Status options around showing ignored files”
  1. 0/6 Teach Status options around showing ignored filesjameson.miller81@gmail.com, Oct 5, 2017
  2. 3/6 Add tests for git status `--ignored=matching`jameson.miller81@gmail.com, Oct 5, 2017
  3. Stefan BellerOct 5, 2017
  4. 5/6 Add tests around status handling of ignored argumentsjameson.miller81@gmail.com, Oct 5, 2017
  5. 6/6 Handle unsupported combination status argumentsjameson.miller81@gmail.com, Oct 5, 2017
  6. Stefan BellerOct 5, 2017
  7. 4/6 Expand support for ignored arguments on statusjameson.miller81@gmail.com, Oct 5, 2017
  8. 1/6 Teach status options around showing ignored filesjameson.miller81@gmail.com, Oct 5, 2017
  9. 2/6 Update documentation for new directory and status logicjameson.miller81@gmail.com, Oct 5, 2017
  10. Stefan BellerOct 5, 2017
  11. Jonathan NiederOct 5, 2017
  12. Jameson MillerOct 5, 2017
  13. 0/5 Teach Status options around showing ignored filesJameson Miller, Oct 11, 2017
  14. 1/5 Teach status options around showing ignored filesJameson Miller, Oct 11, 2017
  15. Junio C HamanoOct 12, 2017
  16. Jameson MillerOct 12, 2017
  17. 2/5 Update documentation for new directory and status logicJameson Miller, Oct 11, 2017
  18. Junio C HamanoOct 12, 2017
  19. Jameson MillerOct 12, 2017
  20. Junio C HamanoOct 13, 2017
  21. 5/5 Add tests around status handling of ignored argumentsJameson Miller, Oct 11, 2017
  22. Junio C HamanoOct 12, 2017
  23. Jameson MillerOct 12, 2017
  24. Junio C HamanoOct 13, 2017
  25. 4/5 Expand support for ignored arguments on statusJameson Miller, Oct 11, 2017
  26. Junio C HamanoOct 12, 2017
  27. 3/5 Add tests for git status `--ignored=matching`Jameson Miller, Oct 11, 2017
  28. Junio C HamanoOct 12, 2017
  29. Jameson MillerOct 12, 2017
  30. 0/4 status: add option to show ignored files differentlyJameson Miller, Oct 19, 2017
  31. 1/4 status: add option to show ignored files differentlyJameson Miller, Oct 19, 2017
  32. Junio C HamanoOct 23, 2017
  33. 2/4 status: report matching ignored and normal untrackedJameson Miller, Oct 19, 2017
  34. 3/4 status: document options to show matching ignored filesJameson Miller, Oct 19, 2017
  35. 4/4 status: test --ignored=matchingJameson Miller, Oct 19, 2017
  36. Stefan BellerOct 19, 2017
  37. Junio C HamanoOct 22, 2017
  38. Junio C HamanoOct 23, 2017
  39. 0/4 status: add option to show ignored files differentlyJameson Miller, Oct 23, 2017
  40. 1/4 status: add option to show ignored files differentlyJameson Miller, Oct 23, 2017
  41. 3/4 status: document options to show matching ignored filesJameson Miller, Oct 23, 2017
  42. 2/4 status: report matching ignored and normal untrackedJameson Miller, Oct 23, 2017
  43. 4/4 status: test ignored modesJameson Miller, Oct 23, 2017
  44. 0/4 status: add option to show ignored files differentlyjameson.miller81@gmail.com, Oct 30, 2017
  45. 3/4 status: document options to show matching ignored filesjameson.miller81@gmail.com, Oct 30, 2017
  46. 4/4 status: test ignored modesjameson.miller81@gmail.com, Oct 30, 2017
  47. 2/4 status: report matching ignored and normal untrackedjameson.miller81@gmail.com, Oct 30, 2017
  48. 1/4 status: add option to show ignored files differentlyjameson.miller81@gmail.com, Oct 30, 2017
  49. Junio C HamanoOct 31, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.