git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/6] Teach Status options around showing ignored files
@ 2017-10-05 20:54 jameson.miller81
  2017-10-05 20:54 ` [PATCH 1/6] Teach status " jameson.miller81
                   ` (10 more replies)
  0 siblings, 11 replies; 49+ messages in thread
From: jameson.miller81 @ 2017-10-05 20:54 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, bmwill, sbeller, Jameson Miller

From: Jameson Miller <jamill@microsoft.com>

This patch series is the second part of [1], which was split into 2
parts. The first part, added an optimization in the directory listing
logic to not scan the contents of ignored directories and was merged
to master with commit 5aaa7fd3. This patch series includes the second
part to expose additional arguments to the --ignored option on the
status command.

This patch series teaches the status command more options to control
which ignored files are reported independently of the which untracked
files are reported by allowing the `--ignored` option to take
additional arguments.  Currently, the shown ignored files are linked
to how untracked files are reported. Both ignored and untracked files
are controlled by arguments to `--untracked-files` option. This makes
it impossible to show all untracked files individually, but show
ignored "files and directories" (that is, ignored directories are
shown as 1 entry, instead of having all contents shown).
    
Our application (Visual Studio) has a specific set of requirements
about how it wants untracked / ignored files reported by git status.
It requires all untracked files listed individually. It would like
ignored files and directories that explicity match an exclude pattern
listed. If an ignored directory matches an exclude pattern, then the
path of the directory should be returned. If a directory does not
match an exclude pattern, but all of its contents are ignored, then we
want the contained files reported instead of the directory.
    
The reason for controlling these behaviors separately is that there
can be a significant performance impact to scanning the contents of
excluded directories. Additionally, knowing whether a directory
explicitly matches an exclude pattern can help the application make
decisions about how to handle the directory. If an ignored directory
itself matches an exclude pattern, then the application will know that
any files underneath the directory must be ignored as well.
    
As a more concrete example, on Windows, Visual Studio creates a bin/
and obj/ directory inside of the project where it writes all .obj and
binary build output files. Normal usage is to explicitly ignore these
2 directory names in the .gitignore file (rather than or in addition
to *.obj). We just want to see the "bin/" and "obj/" paths regardless
of which "--untracked-files" flag is passed in. Additionally, if we
know the bin/ and obj/ directories are ignored, then we can ignore any
files changes we notice underneath these paths, as we know they do not
affect the output of stats.

[PATCH v1 0/1] Teach status to show ignored directories
[1] https://public-inbox.org/git/20170810184936.239542-1-jamill@microsoft.com/

Jameson Miller (6):
  Teach status options around showing ignored files
  Update documentation for new directory and status logic
  Add tests for git status `--ignored=matching`
  Expand support for ignored arguments on status
  Add tests around status handling of ignored arguments
  Handle unsupported combination status arguments

 Documentation/git-status.txt                      |  22 ++-
 Documentation/technical/api-directory-listing.txt |  28 +++-
 builtin/commit.c                                  |  32 +++-
 dir.c                                             |  44 ++++-
 dir.h                                             |   3 +-
 t/t7519-ignored-mode.sh                           | 195 ++++++++++++++++++++++
 wt-status.c                                       |  11 +-
 wt-status.h                                       |   8 +-
 8 files changed, 325 insertions(+), 18 deletions(-)
 create mode 100755 t/t7519-ignored-mode.sh

-- 
2.13.6


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

* [PATCH 1/6] Teach status options around showing ignored files
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
@ 2017-10-05 20:54 ` jameson.miller81
  2017-10-05 20:54 ` [PATCH 2/6] Update documentation for new directory and status logic jameson.miller81
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 49+ messages in thread
From: jameson.miller81 @ 2017-10-05 20:54 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, bmwill, sbeller, Jameson Miller

From: Jameson Miller <jamill@microsoft.com>

This change teaches the status command more options to control which
ignored files are reported independently of the which untracked files
are reported by allowing the `--ignored` option to take additional
arguments.  Currently, the shown ignored files are linked to how
untracked files are reported. Both ignored and untracked files are
controlled by arguments to `--untracked-files` option. This makes it
impossible to show all untracked files individually, but show ignored
"files and directories" (that is, ignored directories are shown as 1
entry, instead of having all contents shown).

Our application (Visual Studio) has a specific set of requirements
about how it wants untracked / ignored files reported by git status.
It requires all untracked files listed individually. It would like
ignored files and directories that explicity match an exclude pattern
listed. If an ignored directory matches an exclude pattern, then the
path of the directory should be returned. If a directory does not
match an exclude pattern, but all of its contents are ignored, then we
want the contained files reported instead of the directory.

The reason for controlling these behaviors separately is that there
can be a significant performance impact to scanning the contents of
excluded directories. Additionally, knowing whether a directory
explicitly matches an exclude pattern can help the application make
decisions about how to handle the directory. If an ignored directory
itself matches an exclude pattern, then the application will know that
any files underneath the directory must be ignored as well.

As a more concrete example, on Windows, Visual Studio creates a bin/
and obj/ directory inside of the project where it writes all .obj and
binary build output files. Normal usage is to explicitly ignore these
2 directory names in the .gitignore file (rather than or in addition
to *.obj). We just want to see the "bin/" and "obj/" paths regardless
of which "--untracked-files" flag is passed in. Additionally, if we
know the bin/ and obj/ directories are ignored, then we can ignore any
files changes we notice underneath these paths, as we know they do not
affect the output of stats.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 builtin/commit.c | 27 +++++++++++++++++++++------
 dir.c            | 24 ++++++++++++++++++++++++
 dir.h            |  3 ++-
 wt-status.c      | 11 ++++++++---
 wt-status.h      |  8 +++++++-
 5 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d75b3805ea..34443b45d3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -118,7 +118,7 @@ static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message;
-static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
+static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit;
 
 /*
@@ -139,7 +139,7 @@ static const char *cleanup_arg;
 static enum commit_whence whence;
 static int sequencer_in_use;
 static int use_editor = 1, include_status = 1;
-static int show_ignored_in_status, have_option_m;
+static int have_option_m;
 static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
@@ -1075,6 +1075,19 @@ static const char *find_author_by_nickname(const char *name)
 	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
 }
 
+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" },
 		{ 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" },
@@ -1383,8 +1398,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	finalize_deferred_config(&s);
 
 	handle_untracked_files_arg(&s);
-	if (show_ignored_in_status)
-		s.show_ignored_files = 1;
+	handle_ignored_arg(&s);
+
 	parse_pathspec(&s.pathspec, 0,
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
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)) {
+
+			/*
+			 * 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).
+			 */
+			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;
+		}
 		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;
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) {
 		dir.flags |= DIR_SHOW_IGNORED_TOO;
-	else
+
+		if (s->show_ignored_mode == SHOW_MATCHING_IGNORED)
+			dir.flags |= DIR_SHOW_IGNORED_TOO_MODE_MATCHING;
+	} 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)
 			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", "");
diff --git a/wt-status.h b/wt-status.h
index 64f4d33ea1..fe27b465e2 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -27,6 +27,12 @@ enum untracked_status_type {
 	SHOW_ALL_UNTRACKED_FILES
 };
 
+enum show_ignored_type {
+	SHOW_NO_IGNORED,
+	SHOW_TRADITIONAL_IGNORED,
+	SHOW_MATCHING_IGNORED,
+};
+
 /* from where does this commit originate */
 enum commit_whence {
 	FROM_COMMIT,     /* normal */
@@ -70,7 +76,7 @@ struct wt_status {
 	int display_comment_prefix;
 	int relative_paths;
 	int submodule_summary;
-	int show_ignored_files;
+	enum show_ignored_type show_ignored_mode;
 	enum untracked_status_type show_untracked_files;
 	const char *ignore_submodule_arg;
 	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];
-- 
2.13.6


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

* [PATCH 2/6] Update documentation for new directory and status logic
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
  2017-10-05 20:54 ` [PATCH 1/6] Teach status " jameson.miller81
@ 2017-10-05 20:54 ` jameson.miller81
  2017-10-05 21:51   ` Stefan Beller
  2017-10-05 20:54 ` [PATCH 3/6] Add tests for git status `--ignored=matching` jameson.miller81
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 49+ messages in thread
From: jameson.miller81 @ 2017-10-05 20:54 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, bmwill, sbeller, Jameson Miller

From: Jameson Miller <jamill@microsoft.com>

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 Documentation/git-status.txt                      | 22 +++++++++++++++++-
 Documentation/technical/api-directory-listing.txt | 28 +++++++++++++++++++----
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 9f3a78a36c..7d1410ff3f 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -97,8 +97,28 @@ configuration variable documented in linkgit:git-config[1].
 	(and suppresses the output of submodule summaries when the config option
 	`status.submoduleSummary` is set).
 
---ignored::
+--ignored[=<mode>]::
 	Show ignored files as well.
++
+The mode parameter is used to specify the handling of ignored files.
+It is optional: it defaults to 'default', and if specified, it must be
+stuck to the option (e.g. '--ignored=default`, but not `--ignored default`).
++
+The possible options are:
++
+	- 'traditional' - Shows ignored files and directories, unless
+			  --untracked-files=all is specifed, in which case
+			  individual files in ignored directories are
+			  displayed.
+	- 'no'	        - Show no ignored files.
+	- 'matching'    - Shows ignored files and directories matching an
+			  ignore pattern.
++
+When 'matching' mode is specified, paths that explicity match an
+ignored pattern are shown. If a directory matches an ignore pattern,
+then it is shown, but not paths contained in the ignored directory. If
+a directory does not match an ignore pattern, but all contents are
+ignored, then the directory is not shown, but all contents are shown.
 
 -z::
 	Terminate entries with NUL, instead of LF.  This implies
diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
index 6c77b4920c..86c981c29c 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -22,16 +22,20 @@ The notable options are:
 
 `flags`::
 
-	A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
+	A bit-field of options:
 
 `DIR_SHOW_IGNORED`:::
 
-	Return just ignored files in `entries[]`, not untracked files.
+	Return just ignored files in `entries[]`, not untracked
+	files. This is flag is mutually exclusive with
+	`DIR_SHOW_IGNORED_TOO`.
 
 `DIR_SHOW_IGNORED_TOO`:::
 
-	Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
-	in addition to untracked files in `entries[]`.
+	Similar to `DIR_SHOW_IGNORED`, but return ignored files in
+	`ignored[]` in addition to untracked files in
+	`entries[]`. This flag is mutually exclusive with
+	`DIR_SHOW_IGNORED`.
 
 `DIR_KEEP_UNTRACKED_CONTENTS`:::
 
@@ -39,6 +43,22 @@ The notable options are:
 	untracked contents of untracked directories are also returned in
 	`entries[]`.
 
+`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
+
+	Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
+	this is set, returns ignored files and directories that match
+	an exclude pattern. If a directory matches an exclude pattern,
+	then the directory is returned and the contained paths are
+	not. A directory that does not match an exclude pattern will
+	not be returned even if all of its contents are ignored. In
+	this case, the contents are returned as individual entries.
+
+If this is set, files and directories
+	that explicity match an ignore pattern are reported. Implicity
+	ignored directories (directories that do not match an ignore
+	pattern, but whose contents are all ignored) are not reported,
+	instead all of the contents are reported.
+
 `DIR_COLLECT_IGNORED`:::
 
 	Special mode for git-add. Return ignored files in `ignored[]` and
-- 
2.13.6


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

* [PATCH 3/6] Add tests for git status `--ignored=matching`
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
  2017-10-05 20:54 ` [PATCH 1/6] Teach status " jameson.miller81
  2017-10-05 20:54 ` [PATCH 2/6] Update documentation for new directory and status logic jameson.miller81
@ 2017-10-05 20:54 ` jameson.miller81
  2017-10-05 21:59   ` Stefan Beller
  2017-10-05 20:54 ` [PATCH 4/6] Expand support for ignored arguments on status jameson.miller81
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 49+ messages in thread
From: jameson.miller81 @ 2017-10-05 20:54 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, bmwill, sbeller, Jameson Miller

From: Jameson Miller <jamill@microsoft.com>

Add tests for new ignored mode (matching) when showing all untracked
files.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 t/t7519-ignored-mode.sh | 132 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100755 t/t7519-ignored-mode.sh

diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
new file mode 100755
index 0000000000..a8c35d1cbc
--- /dev/null
+++ b/t/t7519-ignored-mode.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+
+test_description='git status collapse ignored'
+
+. ./test-lib.sh
+
+# commit initial ignore file
+test_expect_success 'setup initial commit and ignore file' '
+	cat >.gitignore <<-\EOF &&
+	*.ign
+	ignored_dir/
+	!*.unignore
+	EOF
+	git add . &&
+	test_tick &&
+	git commit -m "Initial commit"
+'
+
+# Test status behavior on folder with ignored files
+test_expect_success 'Verify behavior of status on folders with ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/ignored/ignored_1.ign
+	! dir/ignored/ignored_2.ign
+	! ignored/ignored_1.ign
+	! ignored/ignored_2.ign
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+# Test status behavior on folder with tracked and ignored files
+test_expect_success 'Verify status on folder with tracked & ignored files' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/tracked_ignored/ignored_1.ign
+	! dir/tracked_ignored/ignored_2.ign
+	! tracked_ignored/ignored_1.ign
+	! tracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p tracked_ignored dir/tracked_ignored &&
+	touch tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		tracked_ignored/ignored_1.ign tracked_ignored/ignored_2.ign \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 \
+		dir/tracked_ignored/ignored_1.ign dir/tracked_ignored/ignored_2.ign &&
+
+	git add tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 &&
+	test_tick &&
+	git commit -m "commit tracked files" &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+# Test status behavior on folder with untracked and ignored files
+test_expect_success 'Verify status matching ignored files on folder with tracked & ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? dir/untracked_ignored/untracked_1
+	? dir/untracked_ignored/untracked_2
+	? expect
+	? output
+	? untracked_ignored/untracked_1
+	? untracked_ignored/untracked_2
+	! dir/untracked_ignored/ignored_1.ign
+	! dir/untracked_ignored/ignored_2.ign
+	! untracked_ignored/ignored_1.ign
+	! untracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p untracked_ignored dir/untracked_ignored &&
+	touch untracked_ignored/untracked_1 untracked_ignored/untracked_2 \
+		untracked_ignored/ignored_1.ign untracked_ignored/ignored_2.ign \
+		dir/untracked_ignored/untracked_1 dir/untracked_ignored/untracked_2 \
+		dir/untracked_ignored/ignored_1.ign dir/untracked_ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+# Test status behavior on ignored folder
+test_expect_success 'Verify status matching ignored files on ignored folder' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+# Test status behavior on ignored folder with tracked file
+test_expect_success 'Verify status behavior on ignored folder containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	test_tick &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_done
-- 
2.13.6


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

* [PATCH 4/6] Expand support for ignored arguments on status
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
                   ` (2 preceding siblings ...)
  2017-10-05 20:54 ` [PATCH 3/6] Add tests for git status `--ignored=matching` jameson.miller81
@ 2017-10-05 20:54 ` jameson.miller81
  2017-10-05 20:54 ` [PATCH 5/6] Add tests around status handling of ignored arguments jameson.miller81
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 49+ messages in thread
From: jameson.miller81 @ 2017-10-05 20:54 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, bmwill, sbeller, Jameson Miller

From: Jameson Miller <jamill@microsoft.com>

Teach status command to handle matching ignored mode when showing
untracked files with the normal option.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 dir.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index b9af87eca9..8636d080b2 100644
--- a/dir.c
+++ b/dir.c
@@ -1585,6 +1585,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 {
 	int exclude;
 	int has_path_in_index = !!index_file_exists(istate, path->buf, path->len, ignore_case);
+	enum path_treatment path_treatment;
 
 	if (dtype == DT_UNKNOWN)
 		dtype = get_dtype(de, istate, path->buf, path->len);
@@ -1631,8 +1632,23 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 		return path_none;
 	case DT_DIR:
 		strbuf_addch(path, '/');
-		return treat_directory(dir, istate, untracked, path->buf, path->len,
-				       baselen, exclude, pathspec);
+		path_treatment = treat_directory(dir, istate, untracked,
+						 path->buf, path->len,
+						 baselen, exclude, pathspec);
+		/*
+		 * If we are only want to return directories that
+		 * match an exclude pattern, and this directories does
+		 * not match an exclude pattern but all contents are
+		 * excluded, then indicate that we should recurse into
+		 * this directory (instead of marking the directory
+		 * itself as an ignored path)
+		 */
+		if (!exclude &&
+		    path_treatment == path_excluded &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO) &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO_MODE_MATCHING))
+			return path_recurse;
+		return path_treatment;
 	case DT_REG:
 	case DT_LNK:
 		return exclude ? path_excluded : path_untracked;
-- 
2.13.6


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

* [PATCH 5/6] Add tests around status handling of ignored arguments
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
                   ` (3 preceding siblings ...)
  2017-10-05 20:54 ` [PATCH 4/6] Expand support for ignored arguments on status jameson.miller81
@ 2017-10-05 20:54 ` jameson.miller81
  2017-10-05 20:54 ` [PATCH 6/6] Handle unsupported combination status arguments jameson.miller81
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 49+ messages in thread
From: jameson.miller81 @ 2017-10-05 20:54 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, bmwill, sbeller, Jameson Miller

From: Jameson Miller <jamill@microsoft.com>

Add tests for status handling of '--ignored=matching` and
`--untracked-files=normal`.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 t/t7519-ignored-mode.sh | 63 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
index a8c35d1cbc..7292ec7d06 100755
--- a/t/t7519-ignored-mode.sh
+++ b/t/t7519-ignored-mode.sh
@@ -129,4 +129,67 @@ test_expect_success 'Verify status behavior on ignored folder containing tracked
 	test_i18ncmp expect output
 '
 
+# Test ignored matching behavior with --untracked=normal
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+# Test ignored matching behavior with --untracked=normal
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+# Test status behavior on ignored folder with tracked file
+test_expect_success 'Verify status behavior on ignored folder containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	test_tick &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
 test_done
-- 
2.13.6


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

* [PATCH 6/6] Handle unsupported combination status arguments
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
                   ` (4 preceding siblings ...)
  2017-10-05 20:54 ` [PATCH 5/6] Add tests around status handling of ignored arguments jameson.miller81
@ 2017-10-05 20:54 ` jameson.miller81
  2017-10-05 22:08   ` Stefan Beller
  2017-10-05 21:16 ` [PATCH 0/6] Teach Status options around showing ignored files Jonathan Nieder
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 49+ messages in thread
From: jameson.miller81 @ 2017-10-05 20:54 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, bmwill, sbeller, Jameson Miller

From: Jameson Miller <jamill@microsoft.com>

It is not clear what the correct behavior should be when you ask for
specific ignored behavior without reporting untracked files. For now,
report this as an unsupported combination of input arguments, so it
can be modified in the future without back compat concerns.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 builtin/commit.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/builtin/commit.c b/builtin/commit.c
index 34443b45d3..7812e106ad 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1400,6 +1400,11 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	handle_untracked_files_arg(&s);
 	handle_ignored_arg(&s);
 
+	/* Check for unsupported combination of args */
+	if (s.show_ignored_mode == SHOW_MATCHING_IGNORED &&
+	    s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
+		die(_("Unsupported combination of ignored and untracked-files arguments"));
+
 	parse_pathspec(&s.pathspec, 0,
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
-- 
2.13.6


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

* Re: [PATCH 0/6] Teach Status options around showing ignored files
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
                   ` (5 preceding siblings ...)
  2017-10-05 20:54 ` [PATCH 6/6] Handle unsupported combination status arguments jameson.miller81
@ 2017-10-05 21:16 ` Jonathan Nieder
  2017-10-05 21:22   ` Jameson Miller
  2017-10-11 13:34 ` [PATCH v2 0/5] " Jameson Miller
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 49+ messages in thread
From: Jonathan Nieder @ 2017-10-05 21:16 UTC (permalink / raw)
  To: jameson.miller81; +Cc: git, gitster, peff, bmwill, sbeller, Jameson Miller

Hi,

jameson.miller81@gmail.com wrote:

> This patch series is the second part of [1], which was split into 2
> parts. The first part, added an optimization in the directory listing
> logic to not scan the contents of ignored directories and was merged
> to master with commit 5aaa7fd3. This patch series includes the second
> part to expose additional arguments to the --ignored option on the
> status command.

Thanks.

> This patch series teaches the status command more options to control
> which ignored files are reported independently of the which untracked
[...]
> Our application (Visual Studio) has a specific set of requirements
> about how it wants untracked / ignored files reported by git status.
[...]
> The reason for controlling these behaviors separately is that there
> can be a significant performance impact to scanning the contents of
[....]
> As a more concrete example, on Windows, Visual Studio creates a bin/
> and obj/ directory inside of the project where it writes all .obj and
[...]

I see this information is also in patch 1/6.  That's a very good
thing, since that makes performance numbers involved more concrete
about which patch brings them about and it becomes part of permanent
history that way --- thanks.

But it took me a while to notice, and before then, I was trying to
read through the cover letter to get an overview of which patches I am
supposed to look at.  For next time, could the cover letter say
something like "See patches 1 and 2 for more details about the
motivation" instead of repeating the commit message content?  That
would save reviewers some time.

Thanks,
Jonathan

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

* Re: [PATCH 0/6] Teach Status options around showing ignored files
  2017-10-05 21:16 ` [PATCH 0/6] Teach Status options around showing ignored files Jonathan Nieder
@ 2017-10-05 21:22   ` Jameson Miller
  0 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-05 21:22 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, gitster, peff, bmwill, sbeller, Jameson Miller



On 10/05/2017 05:16 PM, Jonathan Nieder wrote:
> Hi,
>
> jameson.miller81@gmail.com wrote:
>
>> This patch series is the second part of [1], which was split into 2
>> parts. The first part, added an optimization in the directory listing
>> logic to not scan the contents of ignored directories and was merged
>> to master with commit 5aaa7fd3. This patch series includes the second
>> part to expose additional arguments to the --ignored option on the
>> status command.
> Thanks.
>
>> This patch series teaches the status command more options to control
>> which ignored files are reported independently of the which untracked
> [...]
>> Our application (Visual Studio) has a specific set of requirements
>> about how it wants untracked / ignored files reported by git status.
> [...]
>> The reason for controlling these behaviors separately is that there
>> can be a significant performance impact to scanning the contents of
> [....]
>> As a more concrete example, on Windows, Visual Studio creates a bin/
>> and obj/ directory inside of the project where it writes all .obj and
> [...]
>
> I see this information is also in patch 1/6.  That's a very good
> thing, since that makes performance numbers involved more concrete
> about which patch brings them about and it becomes part of permanent
> history that way --- thanks.
>
> But it took me a while to notice, and before then, I was trying to
> read through the cover letter to get an overview of which patches I am
> supposed to look at.  For next time, could the cover letter say
> something like "See patches 1 and 2 for more details about the
> motivation" instead of repeating the commit message content?  That
> would save reviewers some time.
Will do - thank you for the feedback!
>
> Thanks,
> Jonathan


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

* Re: [PATCH 2/6] Update documentation for new directory and status logic
  2017-10-05 20:54 ` [PATCH 2/6] Update documentation for new directory and status logic jameson.miller81
@ 2017-10-05 21:51   ` Stefan Beller
  0 siblings, 0 replies; 49+ messages in thread
From: Stefan Beller @ 2017-10-05 21:51 UTC (permalink / raw)
  To: Jameson Miller
  Cc: git@vger.kernel.org, Junio C Hamano, Jeff King, Brandon Williams,
	Jameson Miller

On Thu, Oct 5, 2017 at 1:54 PM,  <jameson.miller81@gmail.com> wrote:
> From: Jameson Miller <jamill@microsoft.com>
>
> Signed-off-by: Jameson Miller <jamill@microsoft.com>
> ---
>  Documentation/git-status.txt                      | 22 +++++++++++++++++-
>  Documentation/technical/api-directory-listing.txt | 28 +++++++++++++++++++----
>  2 files changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index 9f3a78a36c..7d1410ff3f 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -97,8 +97,28 @@ configuration variable documented in linkgit:git-config[1].
>         (and suppresses the output of submodule summaries when the config option
>         `status.submoduleSummary` is set).
>
> ---ignored::
> +--ignored[=<mode>]::
>         Show ignored files as well.
> ++
> +The mode parameter is used to specify the handling of ignored files.
> +It is optional: it defaults to 'default', and if specified, it must be
> +stuck to the option (e.g. '--ignored=default`, but not `--ignored default`).

Is there a rationale for not accepting `--ignored default`?
(It's just the way OPTION_STRING inputs seem to work,
but in other cases [c.f. man git gc, search "--prune=" ] this is
just implied). Or rather: no need to spell this out explicitly IMHO,
as that draws the users attention to it, which might be confusing.

> ++
> +The possible options are:
> ++
> +       - 'traditional' - Shows ignored files and directories, unless
> +                         --untracked-files=all is specifed, in which case
> +                         individual files in ignored directories are
> +                         displayed.
> +       - 'no'          - Show no ignored files.
> +       - 'matching'    - Shows ignored files and directories matching an
> +                         ignore pattern.
> ++
> +When 'matching' mode is specified, paths that explicity match an
> +ignored pattern are shown. If a directory matches an ignore pattern,
> +then it is shown, but not paths contained in the ignored directory. If
> +a directory does not match an ignore pattern, but all contents are
> +ignored, then the directory is not shown, but all contents are shown.
>
>  -z::
>         Terminate entries with NUL, instead of LF.  This implies
> diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
> index 6c77b4920c..86c981c29c 100644
> --- a/Documentation/technical/api-directory-listing.txt
> +++ b/Documentation/technical/api-directory-listing.txt
> @@ -22,16 +22,20 @@ The notable options are:
>
>  `flags`::
>
> -       A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
> +       A bit-field of options:
>
>  `DIR_SHOW_IGNORED`:::
>
> -       Return just ignored files in `entries[]`, not untracked files.
> +       Return just ignored files in `entries[]`, not untracked
> +       files. This is flag is mutually exclusive with

"is flag is" ?


> +       `DIR_SHOW_IGNORED_TOO`.
>
>  `DIR_SHOW_IGNORED_TOO`:::
>
> -       Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
> -       in addition to untracked files in `entries[]`.
> +       Similar to `DIR_SHOW_IGNORED`, but return ignored files in
> +       `ignored[]` in addition to untracked files in
> +       `entries[]`. This flag is mutually exclusive with
> +       `DIR_SHOW_IGNORED`.
>
>  `DIR_KEEP_UNTRACKED_CONTENTS`:::
>
> @@ -39,6 +43,22 @@ The notable options are:
>         untracked contents of untracked directories are also returned in
>         `entries[]`.
>
> +`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
> +
> +       Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
> +       this is set, returns ignored files and directories that match
> +       an exclude pattern. If a directory matches an exclude pattern,
> +       then the directory is returned and the contained paths are
> +       not. A directory that does not match an exclude pattern will
> +       not be returned even if all of its contents are ignored. In
> +       this case, the contents are returned as individual entries.
> +

I think to make the asciidoc happy, you'd put a '+ LF' as paragraph
delimiter and the subsequent paragraphs are not indented.
C.f. Documentation/git-gc.txt "--auto".
Oh, screw that. It turns out, this place is the only place in our docs
where we use triple colons. So I don't know what I am talking about.

> +If this is set, files and directories
> +       that explicity match an ignore pattern are reported. Implicity
> +       ignored directories (directories that do not match an ignore
> +       pattern, but whose contents are all ignored) are not reported,
> +       instead all of the contents are reported.
> +
>  `DIR_COLLECT_IGNORED`:::
>
>         Special mode for git-add. Return ignored files in `ignored[]` and
> --
> 2.13.6
>

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

* Re: [PATCH 3/6] Add tests for git status `--ignored=matching`
  2017-10-05 20:54 ` [PATCH 3/6] Add tests for git status `--ignored=matching` jameson.miller81
@ 2017-10-05 21:59   ` Stefan Beller
  0 siblings, 0 replies; 49+ messages in thread
From: Stefan Beller @ 2017-10-05 21:59 UTC (permalink / raw)
  To: Jameson Miller
  Cc: git@vger.kernel.org, Junio C Hamano, Jeff King, Brandon Williams,
	Jameson Miller

On Thu, Oct 5, 2017 at 1:54 PM,  <jameson.miller81@gmail.com> wrote:
> From: Jameson Miller <jamill@microsoft.com>
>
> Add tests for new ignored mode (matching) when showing all untracked
> files.
>
> Signed-off-by: Jameson Miller <jamill@microsoft.com>
> ---
>  t/t7519-ignored-mode.sh | 132 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100755 t/t7519-ignored-mode.sh
>
> diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
> new file mode 100755
> index 0000000000..a8c35d1cbc
> --- /dev/null
> +++ b/t/t7519-ignored-mode.sh
> @@ -0,0 +1,132 @@
> +#!/bin/sh
> +
> +test_description='git status collapse ignored'
> +
> +. ./test-lib.sh
> +
> +# commit initial ignore file
> +test_expect_success 'setup initial commit and ignore file' '

Here and in the following tests, I have the impression that the
leading comment says the same as the test title, so you could
omit the comments, putting all effort into a well word smithed
test title.


> +       cat >.gitignore <<-\EOF &&
> +       *.ign
> +       ignored_dir/
> +       !*.unignore
> +       EOF
> +       git add . &&
> +       test_tick &&

How is timing relevant to these test?

Thanks,
Stefan

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

* Re: [PATCH 6/6] Handle unsupported combination status arguments
  2017-10-05 20:54 ` [PATCH 6/6] Handle unsupported combination status arguments jameson.miller81
@ 2017-10-05 22:08   ` Stefan Beller
  0 siblings, 0 replies; 49+ messages in thread
From: Stefan Beller @ 2017-10-05 22:08 UTC (permalink / raw)
  To: Jameson Miller
  Cc: git@vger.kernel.org, Junio C Hamano, Jeff King, Brandon Williams,
	Jameson Miller

On Thu, Oct 5, 2017 at 1:54 PM,  <jameson.miller81@gmail.com> wrote:
> From: Jameson Miller <jamill@microsoft.com>
>
> It is not clear what the correct behavior should be when you ask for
> specific ignored behavior without reporting untracked files. For now,
> report this as an unsupported combination of input arguments, so it
> can be modified in the future without back compat concerns.

Is there a rationale to put this as an extra commit at the end,
or could this be squashed into the first commit as well?

>
> Signed-off-by: Jameson Miller <jamill@microsoft.com>
> ---
>  builtin/commit.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 34443b45d3..7812e106ad 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1400,6 +1400,11 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>         handle_untracked_files_arg(&s);
>         handle_ignored_arg(&s);
>
> +       /* Check for unsupported combination of args */

Oh, this is the first check that we add here for unsupported combinations.
How much value does this comment bring to the future reader?

> +       if (s.show_ignored_mode == SHOW_MATCHING_IGNORED &&
> +           s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
> +               die(_("Unsupported combination of ignored and untracked-files arguments"));

Thanks for taking care of the corner case!
Stefan

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

* [PATCH v2 0/5] Teach Status options around showing ignored files
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
                   ` (6 preceding siblings ...)
  2017-10-05 21:16 ` [PATCH 0/6] Teach Status options around showing ignored files Jonathan Nieder
@ 2017-10-11 13:34 ` Jameson Miller
  2017-10-11 13:35   ` [PATCH v2 1/5] Teach status " Jameson Miller
                     ` (5 more replies)
  2017-10-19 16:05 ` [PATCH v3 0/4] status: add option to show ignored files differently Jameson Miller
                   ` (2 subsequent siblings)
  10 siblings, 6 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-11 13:34 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Changes in v2:

Addressed review comments from the original series:

* Made fixes / simplifications suggested for documentation

* Removed test_tick from test scripts

* Merged 2 commits (as suggested)

Jameson Miller (5):
  Teach status options around showing ignored files
  Update documentation for new directory and status logic
  Add tests for git status `--ignored=matching`
  Expand support for ignored arguments on status
  Add tests around status handling of ignored arguments

 Documentation/git-status.txt                      |  21 ++-
 Documentation/technical/api-directory-listing.txt |  27 +++-
 builtin/commit.c                                  |  31 +++-
 dir.c                                             |  44 +++++-
 dir.h                                             |   3 +-
 t/t7519-ignored-mode.sh                           | 183 ++++++++++++++++++++++
 wt-status.c                                       |  11 +-
 wt-status.h                                       |   8 +-
 8 files changed, 310 insertions(+), 18 deletions(-)
 create mode 100755 t/t7519-ignored-mode.sh

-- 
2.13.6


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

* [PATCH v2 1/5] Teach status options around showing ignored files
  2017-10-11 13:34 ` [PATCH v2 0/5] " Jameson Miller
@ 2017-10-11 13:35   ` Jameson Miller
  2017-10-12  2:49     ` Junio C Hamano
  2017-10-11 13:35   ` [PATCH v2 2/5] Update documentation for new directory and status logic Jameson Miller
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 49+ messages in thread
From: Jameson Miller @ 2017-10-11 13:35 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

This change teaches the status command more options to control which
ignored files are reported independently of the which untracked files
are reported by allowing the `--ignored` option to take additional
arguments.  Currently, the shown ignored files are linked to how
untracked files are reported. Both ignored and untracked files are
controlled by arguments to `--untracked-files` option. This makes it
impossible to show all untracked files individually, but show ignored
"files and directories" (that is, ignored directories are shown as 1
entry, instead of having all contents shown).

Our application (Visual Studio) has a specific set of requirements
about how it wants untracked / ignored files reported by git status.
It requires all untracked files listed individually. It would like
ignored files and directories that explicity match an exclude pattern
listed. If an ignored directory matches an exclude pattern, then the
path of the directory should be returned. If a directory does not
match an exclude pattern, but all of its contents are ignored, then we
want the contained files reported instead of the directory.

The reason for controlling these behaviors separately is that there
can be a significant performance impact to scanning the contents of
excluded directories. Additionally, knowing whether a directory
explicitly matches an exclude pattern can help the application make
decisions about how to handle the directory. If an ignored directory
itself matches an exclude pattern, then the application will know that
any files underneath the directory must be ignored as well.

As a more concrete example, on Windows, Visual Studio creates a bin/
and obj/ directory inside of the project where it writes all .obj and
binary build output files. Normal usage is to explicitly ignore these
2 directory names in the .gitignore file (rather than or in addition
to *.obj). We just want to see the "bin/" and "obj/" paths regardless
of which "--untracked-files" flag is passed in. Additionally, if we
know the bin/ and obj/ directories are ignored, then we can ignore any
files changes we notice underneath these paths, as we know they do not
affect the output of stats.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 builtin/commit.c | 31 +++++++++++++++++++++++++------
 dir.c            | 24 ++++++++++++++++++++++++
 dir.h            |  3 ++-
 wt-status.c      | 11 ++++++++---
 wt-status.h      |  8 +++++++-
 5 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d75b3805ea..98d84d0277 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -118,7 +118,7 @@ static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message;
-static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
+static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit;
 
 /*
@@ -139,7 +139,7 @@ static const char *cleanup_arg;
 static enum commit_whence whence;
 static int sequencer_in_use;
 static int use_editor = 1, include_status = 1;
-static int show_ignored_in_status, have_option_m;
+static int have_option_m;
 static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
@@ -1075,6 +1075,19 @@ static const char *find_author_by_nickname(const char *name)
 	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
 }
 
+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" },
 		{ 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" },
@@ -1383,8 +1398,12 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	finalize_deferred_config(&s);
 
 	handle_untracked_files_arg(&s);
-	if (show_ignored_in_status)
-		s.show_ignored_files = 1;
+	handle_ignored_arg(&s);
+
+	if (s.show_ignored_mode == SHOW_MATCHING_IGNORED &&
+	    s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
+		die(_("Unsupported combination of ignored and untracked-files arguments"));
+
 	parse_pathspec(&s.pathspec, 0,
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
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)) {
+
+			/*
+			 * 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).
+			 */
+			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;
+		}
 		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;
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) {
 		dir.flags |= DIR_SHOW_IGNORED_TOO;
-	else
+
+		if (s->show_ignored_mode == SHOW_MATCHING_IGNORED)
+			dir.flags |= DIR_SHOW_IGNORED_TOO_MODE_MATCHING;
+	} 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)
 			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", "");
diff --git a/wt-status.h b/wt-status.h
index 64f4d33ea1..fe27b465e2 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -27,6 +27,12 @@ enum untracked_status_type {
 	SHOW_ALL_UNTRACKED_FILES
 };
 
+enum show_ignored_type {
+	SHOW_NO_IGNORED,
+	SHOW_TRADITIONAL_IGNORED,
+	SHOW_MATCHING_IGNORED,
+};
+
 /* from where does this commit originate */
 enum commit_whence {
 	FROM_COMMIT,     /* normal */
@@ -70,7 +76,7 @@ struct wt_status {
 	int display_comment_prefix;
 	int relative_paths;
 	int submodule_summary;
-	int show_ignored_files;
+	enum show_ignored_type show_ignored_mode;
 	enum untracked_status_type show_untracked_files;
 	const char *ignore_submodule_arg;
 	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];
-- 
2.13.6


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

* [PATCH v2 2/5] Update documentation for new directory and status logic
  2017-10-11 13:34 ` [PATCH v2 0/5] " Jameson Miller
  2017-10-11 13:35   ` [PATCH v2 1/5] Teach status " Jameson Miller
@ 2017-10-11 13:35   ` Jameson Miller
  2017-10-12  2:55     ` Junio C Hamano
  2017-10-11 13:35   ` [PATCH v2 3/5] Add tests for git status `--ignored=matching` Jameson Miller
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 49+ messages in thread
From: Jameson Miller @ 2017-10-11 13:35 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 Documentation/git-status.txt                      | 21 +++++++++++++++++-
 Documentation/technical/api-directory-listing.txt | 27 +++++++++++++++++++----
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 9f3a78a36c..fc282e0a92 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -97,8 +97,27 @@ configuration variable documented in linkgit:git-config[1].
 	(and suppresses the output of submodule summaries when the config option
 	`status.submoduleSummary` is set).
 
---ignored::
+--ignored[=<mode>]::
 	Show ignored files as well.
++
+The mode parameter is used to specify the handling of ignored files.
+It is optional: it defaults to 'traditional'.
++
+The possible options are:
++
+	- 'traditional' - Shows ignored files and directories, unless
+			  --untracked-files=all is specifed, in which case
+			  individual files in ignored directories are
+			  displayed.
+	- 'no'	        - Show no ignored files.
+	- 'matching'    - Shows ignored files and directories matching an
+			  ignore pattern.
++
+When 'matching' mode is specified, paths that explicity match an
+ignored pattern are shown. If a directory matches an ignore pattern,
+then it is shown, but not paths contained in the ignored directory. If
+a directory does not match an ignore pattern, but all contents are
+ignored, then the directory is not shown, but all contents are shown.
 
 -z::
 	Terminate entries with NUL, instead of LF.  This implies
diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
index 6c77b4920c..7fae00f44f 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -22,16 +22,20 @@ The notable options are:
 
 `flags`::
 
-	A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
+	A bit-field of options:
 
 `DIR_SHOW_IGNORED`:::
 
-	Return just ignored files in `entries[]`, not untracked files.
+	Return just ignored files in `entries[]`, not untracked
+	files. This flag is mutually exclusive with
+	`DIR_SHOW_IGNORED_TOO`.
 
 `DIR_SHOW_IGNORED_TOO`:::
 
-	Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
-	in addition to untracked files in `entries[]`.
+	Similar to `DIR_SHOW_IGNORED`, but return ignored files in
+	`ignored[]` in addition to untracked files in
+	`entries[]`. This flag is mutually exclusive with
+	`DIR_SHOW_IGNORED`.
 
 `DIR_KEEP_UNTRACKED_CONTENTS`:::
 
@@ -39,6 +43,21 @@ The notable options are:
 	untracked contents of untracked directories are also returned in
 	`entries[]`.
 
+`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
+
+	Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
+	this is set, returns ignored files and directories that match
+	an exclude pattern. If a directory matches an exclude pattern,
+	then the directory is returned and the contained paths are
+	not. A directory that does not match an exclude pattern will
+	not be returned even if all of its contents are ignored. In
+	this case, the contents are returned as individual entries.
++
+If this is set, files and directories that explicity match an ignore
+pattern are reported. Implicity ignored directories (directories that
+do not match an ignore pattern, but whose contents are all ignored)
+are not reported, instead all of the contents are reported.
+
 `DIR_COLLECT_IGNORED`:::
 
 	Special mode for git-add. Return ignored files in `ignored[]` and
-- 
2.13.6


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

* [PATCH v2 3/5] Add tests for git status `--ignored=matching`
  2017-10-11 13:34 ` [PATCH v2 0/5] " Jameson Miller
  2017-10-11 13:35   ` [PATCH v2 1/5] Teach status " Jameson Miller
  2017-10-11 13:35   ` [PATCH v2 2/5] Update documentation for new directory and status logic Jameson Miller
@ 2017-10-11 13:35   ` Jameson Miller
  2017-10-11 13:35   ` [PATCH v2 4/5] Expand support for ignored arguments on status Jameson Miller
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-11 13:35 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Add tests for new ignored mode (matching) when showing all untracked
files.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 t/t7519-ignored-mode.sh | 125 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)
 create mode 100755 t/t7519-ignored-mode.sh

diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
new file mode 100755
index 0000000000..76e91427b0
--- /dev/null
+++ b/t/t7519-ignored-mode.sh
@@ -0,0 +1,125 @@
+#!/bin/sh
+
+test_description='git status collapse ignored'
+
+. ./test-lib.sh
+
+# commit initial ignore file
+test_expect_success 'setup initial commit and ignore file' '
+	cat >.gitignore <<-\EOF &&
+	*.ign
+	ignored_dir/
+	!*.unignore
+	EOF
+	git add . &&
+	git commit -m "Initial commit"
+'
+
+test_expect_success 'Verify behavior of status on folders with ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/ignored/ignored_1.ign
+	! dir/ignored/ignored_2.ign
+	! ignored/ignored_1.ign
+	! ignored/ignored_2.ign
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on folder with tracked & ignored files' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/tracked_ignored/ignored_1.ign
+	! dir/tracked_ignored/ignored_2.ign
+	! tracked_ignored/ignored_1.ign
+	! tracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p tracked_ignored dir/tracked_ignored &&
+	touch tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		tracked_ignored/ignored_1.ign tracked_ignored/ignored_2.ign \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 \
+		dir/tracked_ignored/ignored_1.ign dir/tracked_ignored/ignored_2.ign &&
+
+	git add tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 &&
+	git commit -m "commit tracked files" &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on folder with untracked and ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? dir/untracked_ignored/untracked_1
+	? dir/untracked_ignored/untracked_2
+	? expect
+	? output
+	? untracked_ignored/untracked_1
+	? untracked_ignored/untracked_2
+	! dir/untracked_ignored/ignored_1.ign
+	! dir/untracked_ignored/ignored_2.ign
+	! untracked_ignored/ignored_1.ign
+	! untracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p untracked_ignored dir/untracked_ignored &&
+	touch untracked_ignored/untracked_1 untracked_ignored/untracked_2 \
+		untracked_ignored/ignored_1.ign untracked_ignored/ignored_2.ign \
+		dir/untracked_ignored/untracked_1 dir/untracked_ignored/untracked_2 \
+		dir/untracked_ignored/ignored_1.ign dir/untracked_ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status matching ignored files on ignored folder' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored folder containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	test_tick &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_done
-- 
2.13.6


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

* [PATCH v2 4/5] Expand support for ignored arguments on status
  2017-10-11 13:34 ` [PATCH v2 0/5] " Jameson Miller
                     ` (2 preceding siblings ...)
  2017-10-11 13:35   ` [PATCH v2 3/5] Add tests for git status `--ignored=matching` Jameson Miller
@ 2017-10-11 13:35   ` Jameson Miller
  2017-10-12  3:58     ` Junio C Hamano
  2017-10-11 13:35   ` [PATCH v2 5/5] Add tests around status handling of ignored arguments Jameson Miller
  2017-10-12  2:33   ` [PATCH v2 0/5] Teach Status options around showing ignored files Junio C Hamano
  5 siblings, 1 reply; 49+ messages in thread
From: Jameson Miller @ 2017-10-11 13:35 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Teach status command to handle matching ignored mode when showing
untracked files with the normal option.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 dir.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index b9af87eca9..8636d080b2 100644
--- a/dir.c
+++ b/dir.c
@@ -1585,6 +1585,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 {
 	int exclude;
 	int has_path_in_index = !!index_file_exists(istate, path->buf, path->len, ignore_case);
+	enum path_treatment path_treatment;
 
 	if (dtype == DT_UNKNOWN)
 		dtype = get_dtype(de, istate, path->buf, path->len);
@@ -1631,8 +1632,23 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 		return path_none;
 	case DT_DIR:
 		strbuf_addch(path, '/');
-		return treat_directory(dir, istate, untracked, path->buf, path->len,
-				       baselen, exclude, pathspec);
+		path_treatment = treat_directory(dir, istate, untracked,
+						 path->buf, path->len,
+						 baselen, exclude, pathspec);
+		/*
+		 * If we are only want to return directories that
+		 * match an exclude pattern, and this directories does
+		 * not match an exclude pattern but all contents are
+		 * excluded, then indicate that we should recurse into
+		 * this directory (instead of marking the directory
+		 * itself as an ignored path)
+		 */
+		if (!exclude &&
+		    path_treatment == path_excluded &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO) &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO_MODE_MATCHING))
+			return path_recurse;
+		return path_treatment;
 	case DT_REG:
 	case DT_LNK:
 		return exclude ? path_excluded : path_untracked;
-- 
2.13.6


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

* [PATCH v2 5/5] Add tests around status handling of ignored arguments
  2017-10-11 13:34 ` [PATCH v2 0/5] " Jameson Miller
                     ` (3 preceding siblings ...)
  2017-10-11 13:35   ` [PATCH v2 4/5] Expand support for ignored arguments on status Jameson Miller
@ 2017-10-11 13:35   ` Jameson Miller
  2017-10-12  4:06     ` Junio C Hamano
  2017-10-12  2:33   ` [PATCH v2 0/5] Teach Status options around showing ignored files Junio C Hamano
  5 siblings, 1 reply; 49+ messages in thread
From: Jameson Miller @ 2017-10-11 13:35 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Add tests for status handling of '--ignored=matching` and
`--untracked-files=normal`.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 t/t7519-ignored-mode.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
index 76e91427b0..6be7701d79 100755
--- a/t/t7519-ignored-mode.sh
+++ b/t/t7519-ignored-mode.sh
@@ -116,10 +116,68 @@ test_expect_success 'Verify status behavior on ignored folder containing tracked
 		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
 		ignored_dir/tracked &&
 	git add -f ignored_dir/tracked &&
-	test_tick &&
 	git commit -m "Force add file in ignored directory" &&
 	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
 	test_i18ncmp expect output
 '
 
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored folder containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
 test_done
-- 
2.13.6


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

* Re: [PATCH v2 0/5] Teach Status options around showing ignored files
  2017-10-11 13:34 ` [PATCH v2 0/5] " Jameson Miller
                     ` (4 preceding siblings ...)
  2017-10-11 13:35   ` [PATCH v2 5/5] Add tests around status handling of ignored arguments Jameson Miller
@ 2017-10-12  2:33   ` Junio C Hamano
  2017-10-12 20:20     ` Jameson Miller
  5 siblings, 1 reply; 49+ messages in thread
From: Junio C Hamano @ 2017-10-12  2:33 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

> Changes in v2:
>
> Addressed review comments from the original series:
>
> * Made fixes / simplifications suggested for documentation
>
> * Removed test_tick from test scripts
>
> * Merged 2 commits (as suggested)
>
> Jameson Miller (5):
>   Teach status options around showing ignored files
>   Update documentation for new directory and status logic
>   Add tests for git status `--ignored=matching`
>   Expand support for ignored arguments on status
>   Add tests around status handling of ignored arguments

Please make sure these titles mix well when they appear together
with changes from other people that are completely unrelated to
them.  Keep in mind that your "git status" is *not* the most
important thing in the world (of course, it is no less important
than others, either).  Perhaps

    status: add new options to show ignored files differently
    status: document logic to show new directory
    status: test --ignored=matching

etc.  Rules of thumb:

 (1) choose "<area>: " prefix appropriately
 (2) keep them short and to the point
 (3) word that follow "<area>: " prefix is not capitalized
 (4) no need for full-stop at the end of the title



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

* Re: [PATCH v2 1/5] Teach status options around showing ignored files
  2017-10-11 13:35   ` [PATCH v2 1/5] Teach status " Jameson Miller
@ 2017-10-12  2:49     ` Junio C Hamano
  2017-10-12 20:15       ` Jameson Miller
  0 siblings, 1 reply; 49+ messages in thread
From: Junio C Hamano @ 2017-10-12  2:49 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

> This change teaches the status command more options to control which
> ignored files are reported independently of the which untracked files
> are reported by allowing the `--ignored` option to take additional
> arguments.  Currently, the shown ignored files are linked to how
> untracked files are reported. Both ignored and untracked files are
> controlled by arguments to `--untracked-files` option. This makes it
> impossible to show all untracked files individually, but show ignored
> "files and directories" (that is, ignored directories are shown as 1
> entry, instead of having all contents shown).

The description makes sense.  And it makes sense to show a directory
known to contain only ignored paths as just one entry, instead of
exploding that to individual files.

> Our application (Visual Studio) has a specific set of requirements
> about how it wants untracked / ignored files reported by git status.

This sentence does not read well.  VS has no obligation to read from
"git status", so there is no "specific set of requirements" that
make us care.  If the output from "status" is insufficient you could
be reading from "ls-files --ignored", for example, if you want more
details than "git status" gives you.

The sentence, and the paragraph that immediately follows it, need a
serious attitude adjustment ;-), even though it is good as read as
an explanation of what the proposed output wants to show.

> The reason for controlling these behaviors separately is that there
> can be a significant performance impact to scanning the contents of
> excluded directories. Additionally, knowing whether a directory
> explicitly matches an exclude pattern can help the application make
> decisions about how to handle the directory. If an ignored directory
> itself matches an exclude pattern, then the application will know that
> any files underneath the directory must be ignored as well.

While the above description taken standalone makes sense, doesn't
the "we want all paths listed, without abbreviated to the directory
and requiring the reader to infer from the fact that aidrectory is
shown that everything in it are ignored" the log message stated
earlier contradict another change you earlier sent, that avoids
scanning a directory that is known to be completely untracked
(i.e. no path under it in the index) and return early once an
untracked file is found in it?

> Signed-off-by: Jameson Miller <jamill@microsoft.com>
> ---
>  builtin/commit.c | 31 +++++++++++++++++++++++++------
>  dir.c            | 24 ++++++++++++++++++++++++
>  dir.h            |  3 ++-
>  wt-status.c      | 11 ++++++++---
>  wt-status.h      |  8 +++++++-
>  5 files changed, 66 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index d75b3805ea..98d84d0277 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -118,7 +118,7 @@ static int edit_flag = -1; /* unspecified */
>  static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
>  static int config_commit_verbose = -1; /* unspecified */
>  static int no_post_rewrite, allow_empty_message;
> -static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
> +static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
>  static char *sign_commit;
>  
>  /*
> @@ -139,7 +139,7 @@ static const char *cleanup_arg;
>  static enum commit_whence whence;
>  static int sequencer_in_use;
>  static int use_editor = 1, include_status = 1;
> -static int show_ignored_in_status, have_option_m;
> +static int have_option_m;
>  static struct strbuf message = STRBUF_INIT;
>  
>  static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
> @@ -1075,6 +1075,19 @@ static const char *find_author_by_nickname(const char *name)
>  	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
>  }
>  
> +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" },
>  		{ 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" },
> @@ -1383,8 +1398,12 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>  	finalize_deferred_config(&s);
>  
>  	handle_untracked_files_arg(&s);
> -	if (show_ignored_in_status)
> -		s.show_ignored_files = 1;
> +	handle_ignored_arg(&s);
> +
> +	if (s.show_ignored_mode == SHOW_MATCHING_IGNORED &&
> +	    s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
> +		die(_("Unsupported combination of ignored and untracked-files arguments"));
> +
>  	parse_pathspec(&s.pathspec, 0,
>  		       PATHSPEC_PREFER_FULL,
>  		       prefix, argv);
> 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)) {
> +
> +			/*
> +			 * 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).
> +			 */
> +			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;
> +		}
>  		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;
> 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) {
>  		dir.flags |= DIR_SHOW_IGNORED_TOO;
> -	else
> +
> +		if (s->show_ignored_mode == SHOW_MATCHING_IGNORED)
> +			dir.flags |= DIR_SHOW_IGNORED_TOO_MODE_MATCHING;
> +	} 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)
>  			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", "");
> diff --git a/wt-status.h b/wt-status.h
> index 64f4d33ea1..fe27b465e2 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -27,6 +27,12 @@ enum untracked_status_type {
>  	SHOW_ALL_UNTRACKED_FILES
>  };
>  
> +enum show_ignored_type {
> +	SHOW_NO_IGNORED,
> +	SHOW_TRADITIONAL_IGNORED,
> +	SHOW_MATCHING_IGNORED,
> +};
> +
>  /* from where does this commit originate */
>  enum commit_whence {
>  	FROM_COMMIT,     /* normal */
> @@ -70,7 +76,7 @@ struct wt_status {
>  	int display_comment_prefix;
>  	int relative_paths;
>  	int submodule_summary;
> -	int show_ignored_files;
> +	enum show_ignored_type show_ignored_mode;
>  	enum untracked_status_type show_untracked_files;
>  	const char *ignore_submodule_arg;
>  	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];

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

* Re: [PATCH v2 2/5] Update documentation for new directory and status logic
  2017-10-11 13:35   ` [PATCH v2 2/5] Update documentation for new directory and status logic Jameson Miller
@ 2017-10-12  2:55     ` Junio C Hamano
  2017-10-12 20:54       ` Jameson Miller
  0 siblings, 1 reply; 49+ messages in thread
From: Junio C Hamano @ 2017-10-12  2:55 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

> Signed-off-by: Jameson Miller <jamill@microsoft.com>
> ---
>  Documentation/git-status.txt                      | 21 +++++++++++++++++-
>  Documentation/technical/api-directory-listing.txt | 27 +++++++++++++++++++----
>  2 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index 9f3a78a36c..fc282e0a92 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -97,8 +97,27 @@ configuration variable documented in linkgit:git-config[1].
>  	(and suppresses the output of submodule summaries when the config option
>  	`status.submoduleSummary` is set).
>  
> ---ignored::
> +--ignored[=<mode>]::
>  	Show ignored files as well.
> ++
> +The mode parameter is used to specify the handling of ignored files.
> +It is optional: it defaults to 'traditional'.
> ++
> +The possible options are:
> ++
> +	- 'traditional' - Shows ignored files and directories, unless
> +			  --untracked-files=all is specifed, in which case
> +			  individual files in ignored directories are
> +			  displayed.
> +	- 'no'	        - Show no ignored files.
> +	- 'matching'    - Shows ignored files and directories matching an
> +			  ignore pattern.
> ++
> +When 'matching' mode is specified, paths that explicity match an
> +ignored pattern are shown. If a directory matches an ignore pattern,
> +then it is shown, but not paths contained in the ignored directory. If
> +a directory does not match an ignore pattern, but all contents are
> +ignored, then the directory is not shown, but all contents are shown.

Well explained.

> diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
> index 6c77b4920c..7fae00f44f 100644
> --- a/Documentation/technical/api-directory-listing.txt
> +++ b/Documentation/technical/api-directory-listing.txt
> @@ -22,16 +22,20 @@ The notable options are:
>  
>  `flags`::
>  
> -	A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
> +	A bit-field of options:
>  
>  `DIR_SHOW_IGNORED`:::
>  
> -	Return just ignored files in `entries[]`, not untracked files.
> +	Return just ignored files in `entries[]`, not untracked
> +	files. This flag is mutually exclusive with
> +	`DIR_SHOW_IGNORED_TOO`.
>  
>  `DIR_SHOW_IGNORED_TOO`:::
>  
> -	Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
> -	in addition to untracked files in `entries[]`.
> +	Similar to `DIR_SHOW_IGNORED`, but return ignored files in
> +	`ignored[]` in addition to untracked files in
> +	`entries[]`. This flag is mutually exclusive with
> +	`DIR_SHOW_IGNORED`.
>  
>  `DIR_KEEP_UNTRACKED_CONTENTS`:::
>  
> @@ -39,6 +43,21 @@ The notable options are:
>  	untracked contents of untracked directories are also returned in
>  	`entries[]`.
>  
> +`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
> +
> +	Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
> +	this is set, returns ignored files and directories that match
> +	an exclude pattern. If a directory matches an exclude pattern,
> +	then the directory is returned and the contained paths are
> +	not. A directory that does not match an exclude pattern will
> +	not be returned even if all of its contents are ignored. In
> +	this case, the contents are returned as individual entries.
> ++
> +If this is set, files and directories that explicity match an ignore
> +pattern are reported. Implicity ignored directories (directories that
> +do not match an ignore pattern, but whose contents are all ignored)
> +are not reported, instead all of the contents are reported.

Makes me wonder if DIR_SHOW_IGNORED* should be splt out into a short
enum.  We have:

 - Do not show ignored ones (0)

 - Collect ignored ones (DIR_SHOW_IGNORED)

 - Collect ignored and untracked ones separately (DIR_SHOW_IGNORED_TOO)

 - Collect ignored and duntracked ones separately, but limit them to
   those mach exclude patterns explicitly (DIR_SHOW_IGNORED_TOO|...MODE_MATCHING)

so we need two bits to fit a 4-possiblity enum.

Then we do not have to worry about saying quirky things like A and B
are incompatible, and C makes sense only when B is set, etc.

>  `DIR_COLLECT_IGNORED`:::
>  
>  	Special mode for git-add. Return ignored files in `ignored[]` and

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

* Re: [PATCH v2 4/5] Expand support for ignored arguments on status
  2017-10-11 13:35   ` [PATCH v2 4/5] Expand support for ignored arguments on status Jameson Miller
@ 2017-10-12  3:58     ` Junio C Hamano
  0 siblings, 0 replies; 49+ messages in thread
From: Junio C Hamano @ 2017-10-12  3:58 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

> Teach status command to handle matching ignored mode when showing
> untracked files with the normal option.
>
> Signed-off-by: Jameson Miller <jamill@microsoft.com>
> ---
>  dir.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index b9af87eca9..8636d080b2 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1585,6 +1585,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
>  {
>  	int exclude;
>  	int has_path_in_index = !!index_file_exists(istate, path->buf, path->len, ignore_case);
> +	enum path_treatment path_treatment;
>  
>  	if (dtype == DT_UNKNOWN)
>  		dtype = get_dtype(de, istate, path->buf, path->len);
> @@ -1631,8 +1632,23 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
>  		return path_none;
>  	case DT_DIR:
>  		strbuf_addch(path, '/');
> -		return treat_directory(dir, istate, untracked, path->buf, path->len,
> -				       baselen, exclude, pathspec);
> +		path_treatment = treat_directory(dir, istate, untracked,
> +						 path->buf, path->len,
> +						 baselen, exclude, pathspec);
> +		/*
> +		 * If we are only want to return directories that
> +		 * match an exclude pattern, and this directories does

s/are //; s/directories/directory/

> +		 * not match an exclude pattern but all contents are
> +		 * excluded, then indicate that we should recurse into
> +		 * this directory (instead of marking the directory
> +		 * itself as an ignored path)
> +		 */
> +		if (!exclude &&
> +		    path_treatment == path_excluded &&
> +		    (dir->flags & DIR_SHOW_IGNORED_TOO) &&
> +		    (dir->flags & DIR_SHOW_IGNORED_TOO_MODE_MATCHING))
> +			return path_recurse;
> +		return path_treatment;

The required change to the code is surprisingly small ;-) and it is
well explained in the comment.  Good job.


>  	case DT_REG:
>  	case DT_LNK:
>  		return exclude ? path_excluded : path_untracked;

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

* Re: [PATCH v2 5/5] Add tests around status handling of ignored arguments
  2017-10-11 13:35   ` [PATCH v2 5/5] Add tests around status handling of ignored arguments Jameson Miller
@ 2017-10-12  4:06     ` Junio C Hamano
  2017-10-12 20:16       ` Jameson Miller
  0 siblings, 1 reply; 49+ messages in thread
From: Junio C Hamano @ 2017-10-12  4:06 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

> Add tests for status handling of '--ignored=matching` and
> `--untracked-files=normal`.
>
> Signed-off-by: Jameson Miller <jamill@microsoft.com>
> ---

Hmph, having some tests in 3/5, changes in 4/5 and even more tests
in 5/5 makes me as a reader a bit confused, as the description for
these two test patches does not make it clear how they are related
and how they are different.  Is it that changes in 1/5 alone does
not fulfill the promise made by documentation added at 2/5 so 3/5
only has tests for behaviour that works with 1/5 alone but is broken
with respect to what 2/5 claims until 4/5 is applied, and these "not
working with 1/5 alone, but works after 4/5" are added in this step?

>  t/t7519-ignored-mode.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
> index 76e91427b0..6be7701d79 100755
> --- a/t/t7519-ignored-mode.sh
> +++ b/t/t7519-ignored-mode.sh
> @@ -116,10 +116,68 @@ test_expect_success 'Verify status behavior on ignored folder containing tracked
>  		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
>  		ignored_dir/tracked &&
>  	git add -f ignored_dir/tracked &&
> -	test_tick &&
>  	git commit -m "Force add file in ignored directory" &&
>  	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
>  	test_i18ncmp expect output
>  '
>  
> +test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
> +	test_when_finished "git clean -fdx" &&
> +	cat >expect <<-\EOF &&
> +	? expect
> +	? output
> +	? untracked_dir/
> +	! ignored_dir/
> +	! ignored_files/ignored_1.ign
> +	! ignored_files/ignored_2.ign
> +	EOF
> +
> +	mkdir ignored_dir ignored_files untracked_dir &&
> +	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
> +		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
> +		untracked_dir/untracked &&
> +	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
> +	test_i18ncmp expect output
> +'
> +
> +test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
> +	test_when_finished "git clean -fdx" &&
> +	cat >expect <<-\EOF &&
> +	? expect
> +	? output
> +	? untracked_dir/
> +	! ignored_dir/
> +	! ignored_files/ignored_1.ign
> +	! ignored_files/ignored_2.ign
> +	EOF
> +
> +	mkdir ignored_dir ignored_files untracked_dir &&
> +	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
> +		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
> +		untracked_dir/untracked &&
> +	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
> +	test_i18ncmp expect output
> +'
> +
> +test_expect_success 'Verify status behavior on ignored folder containing tracked file' '
> +	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
> +	cat >expect <<-\EOF &&
> +	? expect
> +	? output
> +	! ignored_dir/ignored_1
> +	! ignored_dir/ignored_1.ign
> +	! ignored_dir/ignored_2
> +	! ignored_dir/ignored_2.ign
> +	EOF
> +
> +	mkdir ignored_dir &&
> +	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
> +		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
> +		ignored_dir/tracked &&
> +	git add -f ignored_dir/tracked &&
> +	git commit -m "Force add file in ignored directory" &&
> +	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
> +	test_i18ncmp expect output
> +'
> +
>  test_done

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

* Re: [PATCH v2 1/5] Teach status options around showing ignored files
  2017-10-12  2:49     ` Junio C Hamano
@ 2017-10-12 20:15       ` Jameson Miller
  0 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-12 20:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: bmwill, git, jamill, peff, sbeller



On 10/11/2017 10:49 PM, Junio C Hamano wrote:
> Jameson Miller <jameson.miller81@gmail.com> writes:
>
>> This change teaches the status command more options to control which
>> ignored files are reported independently of the which untracked files
>> are reported by allowing the `--ignored` option to take additional
>> arguments.  Currently, the shown ignored files are linked to how
>> untracked files are reported. Both ignored and untracked files are
>> controlled by arguments to `--untracked-files` option. This makes it
>> impossible to show all untracked files individually, but show ignored
>> "files and directories" (that is, ignored directories are shown as 1
>> entry, instead of having all contents shown).
> The description makes sense.  And it makes sense to show a directory
> known to contain only ignored paths as just one entry, instead of
> exploding that to individual files.
>
>> Our application (Visual Studio) has a specific set of requirements
>> about how it wants untracked / ignored files reported by git status.
> This sentence does not read well.  VS has no obligation to read from
> "git status", so there is no "specific set of requirements" that
> make us care.  If the output from "status" is insufficient you could
> be reading from "ls-files --ignored", for example, if you want more
> details than "git status" gives you.
>
> The sentence, and the paragraph that immediately follows it, need a
> serious attitude adjustment ;-), even though it is good as read as
> an explanation of what the proposed output wants to show.

It was not my intention to have this paragraph come across this
way. I meant to express the ideal format of data that our
application would like (from whatever source) as motivation for
why we are proposing these changes. I will reword this
paragraph to remove any unintended implication otherwise.

>> The reason for controlling these behaviors separately is that there
>> can be a significant performance impact to scanning the contents of
>> excluded directories. Additionally, knowing whether a directory
>> explicitly matches an exclude pattern can help the application make
>> decisions about how to handle the directory. If an ignored directory
>> itself matches an exclude pattern, then the application will know that
>> any files underneath the directory must be ignored as well.
> While the above description taken standalone makes sense, doesn't
> the "we want all paths listed, without abbreviated to the directory
> and requiring the reader to infer from the fact that aidrectory is
> shown that everything in it are ignored" the log message stated
> earlier contradict another change you earlier sent, that avoids
> scanning a directory that is known to be completely untracked
> (i.e. no path under it in the index) and return early once an
> untracked file is found in it?

My first set of changes introduced a perf optimization without
any functional changes. The perf optimization was to avoid
scanning a directory that is known to be ignored (i.e no path
under it in the index and the directory matches an exclude
pattern). It returns early once any file is found in it. Any file
found must be ignored, as it is contained in an ignored
directory.

This second set of changes is to allow optional decoupling of how
ignored and untracked items are reported.

>
>> Signed-off-by: Jameson Miller <jamill@microsoft.com>
>> ---
>>   builtin/commit.c | 31 +++++++++++++++++++++++++------
>>   dir.c            | 24 ++++++++++++++++++++++++
>>   dir.h            |  3 ++-
>>   wt-status.c      | 11 ++++++++---
>>   wt-status.h      |  8 +++++++-
>>   5 files changed, 66 insertions(+), 11 deletions(-)
>>
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index d75b3805ea..98d84d0277 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -118,7 +118,7 @@ static int edit_flag = -1; /* unspecified */
>>   static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
>>   static int config_commit_verbose = -1; /* unspecified */
>>   static int no_post_rewrite, allow_empty_message;
>> -static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
>> +static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
>>   static char *sign_commit;
>>   
>>   /*
>> @@ -139,7 +139,7 @@ static const char *cleanup_arg;
>>   static enum commit_whence whence;
>>   static int sequencer_in_use;
>>   static int use_editor = 1, include_status = 1;
>> -static int show_ignored_in_status, have_option_m;
>> +static int have_option_m;
>>   static struct strbuf message = STRBUF_INIT;
>>   
>>   static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
>> @@ -1075,6 +1075,19 @@ static const char *find_author_by_nickname(const char *name)
>>   	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
>>   }
>>   
>> +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" },
>>   		{ 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" },
>> @@ -1383,8 +1398,12 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>>   	finalize_deferred_config(&s);
>>   
>>   	handle_untracked_files_arg(&s);
>> -	if (show_ignored_in_status)
>> -		s.show_ignored_files = 1;
>> +	handle_ignored_arg(&s);
>> +
>> +	if (s.show_ignored_mode == SHOW_MATCHING_IGNORED &&
>> +	    s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
>> +		die(_("Unsupported combination of ignored and untracked-files arguments"));
>> +
>>   	parse_pathspec(&s.pathspec, 0,
>>   		       PATHSPEC_PREFER_FULL,
>>   		       prefix, argv);
>> 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)) {
>> +
>> +			/*
>> +			 * 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).
>> +			 */
>> +			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;
>> +		}
>>   		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;
>> 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) {
>>   		dir.flags |= DIR_SHOW_IGNORED_TOO;
>> -	else
>> +
>> +		if (s->show_ignored_mode == SHOW_MATCHING_IGNORED)
>> +			dir.flags |= DIR_SHOW_IGNORED_TOO_MODE_MATCHING;
>> +	} 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)
>>   			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", "");
>> diff --git a/wt-status.h b/wt-status.h
>> index 64f4d33ea1..fe27b465e2 100644
>> --- a/wt-status.h
>> +++ b/wt-status.h
>> @@ -27,6 +27,12 @@ enum untracked_status_type {
>>   	SHOW_ALL_UNTRACKED_FILES
>>   };
>>   
>> +enum show_ignored_type {
>> +	SHOW_NO_IGNORED,
>> +	SHOW_TRADITIONAL_IGNORED,
>> +	SHOW_MATCHING_IGNORED,
>> +};
>> +
>>   /* from where does this commit originate */
>>   enum commit_whence {
>>   	FROM_COMMIT,     /* normal */
>> @@ -70,7 +76,7 @@ struct wt_status {
>>   	int display_comment_prefix;
>>   	int relative_paths;
>>   	int submodule_summary;
>> -	int show_ignored_files;
>> +	enum show_ignored_type show_ignored_mode;
>>   	enum untracked_status_type show_untracked_files;
>>   	const char *ignore_submodule_arg;
>>   	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];


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

* Re: [PATCH v2 5/5] Add tests around status handling of ignored arguments
  2017-10-12  4:06     ` Junio C Hamano
@ 2017-10-12 20:16       ` Jameson Miller
  2017-10-13  0:49         ` Junio C Hamano
  0 siblings, 1 reply; 49+ messages in thread
From: Jameson Miller @ 2017-10-12 20:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: bmwill, git, jamill, peff, sbeller



On 10/12/2017 12:06 AM, Junio C Hamano wrote:
> Jameson Miller <jameson.miller81@gmail.com> writes:
>
>> Add tests for status handling of '--ignored=matching` and
>> `--untracked-files=normal`.
>>
>> Signed-off-by: Jameson Miller <jamill@microsoft.com>
>> ---
> Hmph, having some tests in 3/5, changes in 4/5 and even more tests
> in 5/5 makes me as a reader a bit confused, as the description for
> these two test patches does not make it clear how they are related
> and how they are different.  Is it that changes in 1/5 alone does
> not fulfill the promise made by documentation added at 2/5 so 3/5
> only has tests for behaviour that works with 1/5 alone but is broken
> with respect to what 2/5 claims until 4/5 is applied, and these "not
> working with 1/5 alone, but works after 4/5" are added in this step?

Correct. The changes in 1/5 are to implement "--ignored=matching"
with "--untracked-files=all" with corresponding tests in 3/5. The
changes in 4/5 are to implement "--ignored=matching"
with "--untracked-files=normal" and the corresponding tests are
in 5/5.

Do you have a preference on how I organized this work? I see
several possible ways to split up this work. Maybe it would be
less confusing to have the implementation in the first two
commits, then 1 commit with all the tests, and then a commit with
the documentation? I think it makes sense to have the logic for
the different flag combinations split into their own commits, but
I am open to any suggestions.

>
>>   t/t7519-ignored-mode.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
>> index 76e91427b0..6be7701d79 100755
>> --- a/t/t7519-ignored-mode.sh
>> +++ b/t/t7519-ignored-mode.sh
>> @@ -116,10 +116,68 @@ test_expect_success 'Verify status behavior on ignored folder containing tracked
>>   		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
>>   		ignored_dir/tracked &&
>>   	git add -f ignored_dir/tracked &&
>> -	test_tick &&
>>   	git commit -m "Force add file in ignored directory" &&
>>   	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
>>   	test_i18ncmp expect output
>>   '
>>   
>> +test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
>> +	test_when_finished "git clean -fdx" &&
>> +	cat >expect <<-\EOF &&
>> +	? expect
>> +	? output
>> +	? untracked_dir/
>> +	! ignored_dir/
>> +	! ignored_files/ignored_1.ign
>> +	! ignored_files/ignored_2.ign
>> +	EOF
>> +
>> +	mkdir ignored_dir ignored_files untracked_dir &&
>> +	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
>> +		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
>> +		untracked_dir/untracked &&
>> +	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
>> +	test_i18ncmp expect output
>> +'
>> +
>> +test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
>> +	test_when_finished "git clean -fdx" &&
>> +	cat >expect <<-\EOF &&
>> +	? expect
>> +	? output
>> +	? untracked_dir/
>> +	! ignored_dir/
>> +	! ignored_files/ignored_1.ign
>> +	! ignored_files/ignored_2.ign
>> +	EOF
>> +
>> +	mkdir ignored_dir ignored_files untracked_dir &&
>> +	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
>> +		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
>> +		untracked_dir/untracked &&
>> +	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
>> +	test_i18ncmp expect output
>> +'
>> +
>> +test_expect_success 'Verify status behavior on ignored folder containing tracked file' '
>> +	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
>> +	cat >expect <<-\EOF &&
>> +	? expect
>> +	? output
>> +	! ignored_dir/ignored_1
>> +	! ignored_dir/ignored_1.ign
>> +	! ignored_dir/ignored_2
>> +	! ignored_dir/ignored_2.ign
>> +	EOF
>> +
>> +	mkdir ignored_dir &&
>> +	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
>> +		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
>> +		ignored_dir/tracked &&
>> +	git add -f ignored_dir/tracked &&
>> +	git commit -m "Force add file in ignored directory" &&
>> +	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
>> +	test_i18ncmp expect output
>> +'
>> +
>>   test_done


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

* Re: [PATCH v2 0/5] Teach Status options around showing ignored files
  2017-10-12  2:33   ` [PATCH v2 0/5] Teach Status options around showing ignored files Junio C Hamano
@ 2017-10-12 20:20     ` Jameson Miller
  0 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-12 20:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: bmwill, git, jamill, peff, sbeller



On 10/11/2017 10:33 PM, Junio C Hamano wrote:
> Jameson Miller <jameson.miller81@gmail.com> writes:
>
>> Changes in v2:
>>
>> Addressed review comments from the original series:
>>
>> * Made fixes / simplifications suggested for documentation
>>
>> * Removed test_tick from test scripts
>>
>> * Merged 2 commits (as suggested)
>>
>> Jameson Miller (5):
>>    Teach status options around showing ignored files
>>    Update documentation for new directory and status logic
>>    Add tests for git status `--ignored=matching`
>>    Expand support for ignored arguments on status
>>    Add tests around status handling of ignored arguments
> Please make sure these titles mix well when they appear together
> with changes from other people that are completely unrelated to
> them.  Keep in mind that your "git status" is *not* the most
> important thing in the world (of course, it is no less important
> than others, either).  Perhaps
>
>      status: add new options to show ignored files differently
>      status: document logic to show new directory
>      status: test --ignored=matching
Thank you for the suggestions - I will update the  commit titles in my 
next round.
>
> etc.  Rules of thumb:
>
>   (1) choose "<area>: " prefix appropriately
>   (2) keep them short and to the point
>   (3) word that follow "<area>: " prefix is not capitalized
>   (4) no need for full-stop at the end of the title
>
>


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

* Re: [PATCH v2 2/5] Update documentation for new directory and status logic
  2017-10-12  2:55     ` Junio C Hamano
@ 2017-10-12 20:54       ` Jameson Miller
  2017-10-13  0:42         ` Junio C Hamano
  0 siblings, 1 reply; 49+ messages in thread
From: Jameson Miller @ 2017-10-12 20:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: bmwill, git, jamill, peff, sbeller



On 10/11/2017 10:55 PM, Junio C Hamano wrote:
> Jameson Miller <jameson.miller81@gmail.com> writes:
>
>> Signed-off-by: Jameson Miller <jamill@microsoft.com>
>> ---
>>   Documentation/git-status.txt                      | 21 +++++++++++++++++-
>>   Documentation/technical/api-directory-listing.txt | 27 +++++++++++++++++++----
>>   2 files changed, 43 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
>> index 9f3a78a36c..fc282e0a92 100644
>> --- a/Documentation/git-status.txt
>> +++ b/Documentation/git-status.txt
>> @@ -97,8 +97,27 @@ configuration variable documented in linkgit:git-config[1].
>>   	(and suppresses the output of submodule summaries when the config option
>>   	`status.submoduleSummary` is set).
>>   
>> ---ignored::
>> +--ignored[=<mode>]::
>>   	Show ignored files as well.
>> ++
>> +The mode parameter is used to specify the handling of ignored files.
>> +It is optional: it defaults to 'traditional'.
>> ++
>> +The possible options are:
>> ++
>> +	- 'traditional' - Shows ignored files and directories, unless
>> +			  --untracked-files=all is specifed, in which case
>> +			  individual files in ignored directories are
>> +			  displayed.
>> +	- 'no'	        - Show no ignored files.
>> +	- 'matching'    - Shows ignored files and directories matching an
>> +			  ignore pattern.
>> ++
>> +When 'matching' mode is specified, paths that explicity match an
>> +ignored pattern are shown. If a directory matches an ignore pattern,
>> +then it is shown, but not paths contained in the ignored directory. If
>> +a directory does not match an ignore pattern, but all contents are
>> +ignored, then the directory is not shown, but all contents are shown.
> Well explained.
>
>> diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
>> index 6c77b4920c..7fae00f44f 100644
>> --- a/Documentation/technical/api-directory-listing.txt
>> +++ b/Documentation/technical/api-directory-listing.txt
>> @@ -22,16 +22,20 @@ The notable options are:
>>   
>>   `flags`::
>>   
>> -	A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
>> +	A bit-field of options:
>>   
>>   `DIR_SHOW_IGNORED`:::
>>   
>> -	Return just ignored files in `entries[]`, not untracked files.
>> +	Return just ignored files in `entries[]`, not untracked
>> +	files. This flag is mutually exclusive with
>> +	`DIR_SHOW_IGNORED_TOO`.
>>   
>>   `DIR_SHOW_IGNORED_TOO`:::
>>   
>> -	Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
>> -	in addition to untracked files in `entries[]`.
>> +	Similar to `DIR_SHOW_IGNORED`, but return ignored files in
>> +	`ignored[]` in addition to untracked files in
>> +	`entries[]`. This flag is mutually exclusive with
>> +	`DIR_SHOW_IGNORED`.
>>   
>>   `DIR_KEEP_UNTRACKED_CONTENTS`:::
>>   
>> @@ -39,6 +43,21 @@ The notable options are:
>>   	untracked contents of untracked directories are also returned in
>>   	`entries[]`.
>>   
>> +`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
>> +
>> +	Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
>> +	this is set, returns ignored files and directories that match
>> +	an exclude pattern. If a directory matches an exclude pattern,
>> +	then the directory is returned and the contained paths are
>> +	not. A directory that does not match an exclude pattern will
>> +	not be returned even if all of its contents are ignored. In
>> +	this case, the contents are returned as individual entries.
>> ++
>> +If this is set, files and directories that explicity match an ignore
>> +pattern are reported. Implicity ignored directories (directories that
>> +do not match an ignore pattern, but whose contents are all ignored)
>> +are not reported, instead all of the contents are reported.
> Makes me wonder if DIR_SHOW_IGNORED* should be splt out into a short
> enum.  We have:
>
>   - Do not show ignored ones (0)
>
>   - Collect ignored ones (DIR_SHOW_IGNORED)
>
>   - Collect ignored and untracked ones separately (DIR_SHOW_IGNORED_TOO)
>
>   - Collect ignored and duntracked ones separately, but limit them to
>     those mach exclude patterns explicitly (DIR_SHOW_IGNORED_TOO|...MODE_MATCHING)
>
> so we need two bits to fit a 4-possiblity enum.
>
> Then we do not have to worry about saying quirky things like A and B
> are incompatible, and C makes sense only when B is set, etc.
I could see a potential for other values for the "show ignored
mode" flags - for example: "NORMAL", "MATCHING", "ALL"... Instead
of making more change at this point in time, how would you feel
about waiting until the next change in this area.

If you would prefer for me to change these enums now, I can do
that.

>
>>   `DIR_COLLECT_IGNORED`:::
>>   
>>   	Special mode for git-add. Return ignored files in `ignored[]` and


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

* Re: [PATCH v2 2/5] Update documentation for new directory and status logic
  2017-10-12 20:54       ` Jameson Miller
@ 2017-10-13  0:42         ` Junio C Hamano
  0 siblings, 0 replies; 49+ messages in thread
From: Junio C Hamano @ 2017-10-13  0:42 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

>>> +If this is set, files and directories that explicity match an ignore
>>> +pattern are reported. Implicity ignored directories (directories that
>>> +do not match an ignore pattern, but whose contents are all ignored)
>>> +are not reported, instead all of the contents are reported.
>> Makes me wonder if DIR_SHOW_IGNORED* should be splt out into a short
>> enum.  We have:
>>
>>   - Do not show ignored ones (0)
>>
>>   - Collect ignored ones (DIR_SHOW_IGNORED)
>>
>>   - Collect ignored and untracked ones separately (DIR_SHOW_IGNORED_TOO)
>>
>>   - Collect ignored and duntracked ones separately, but limit them to
>>     those mach exclude patterns explicitly (DIR_SHOW_IGNORED_TOO|...MODE_MATCHING)
>>
>> so we need two bits to fit a 4-possiblity enum.
>>
>> Then we do not have to worry about saying quirky things like A and B
>> are incompatible, and C makes sense only when B is set, etc.
> I could see a potential for other values for the "show ignored
> mode" flags - for example: "NORMAL", "MATCHING", "ALL"... Instead
> of making more change at this point in time, how would you feel
> about waiting until the next change in this area.
>
> If you would prefer for me to change these enums now, I can do
> that.

"Makes me wonder" was just that.  I was made to wonder.  I did not
have strong opinions either way.  You thought about the area of this
code longer than I did, so I do not mind you picking the course of
action that is best for the project, and if you think it is better
to wait until we know more about the vocabulary we want to support
before we restructure these flags, that is fine by me.

Thanks.



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

* Re: [PATCH v2 5/5] Add tests around status handling of ignored arguments
  2017-10-12 20:16       ` Jameson Miller
@ 2017-10-13  0:49         ` Junio C Hamano
  0 siblings, 0 replies; 49+ messages in thread
From: Junio C Hamano @ 2017-10-13  0:49 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

>> Hmph, having some tests in 3/5, changes in 4/5 and even more tests
>> in 5/5 makes me as a reader a bit confused, as the description for
>> these two test patches does not make it clear how they are related
>> and how they are different.  Is it that changes in 1/5 alone does
>> not fulfill the promise made by documentation added at 2/5 so 3/5
>> only has tests for behaviour that works with 1/5 alone but is broken
>> with respect to what 2/5 claims until 4/5 is applied, and these "not
>> working with 1/5 alone, but works after 4/5" are added in this step?
>
> Correct. The changes in 1/5 are to implement "--ignored=matching"
> with "--untracked-files=all" with corresponding tests in 3/5. The
> changes in 4/5 are to implement "--ignored=matching"
> with "--untracked-files=normal" and the corresponding tests are
> in 5/5.
>
> Do you have a preference on how I organized this work? I see
> several possible ways to split up this work. Maybe it would be
> less confusing to have the implementation in the first two
> commits, then 1 commit with all the tests, and then a commit with
> the documentation? I think it makes sense to have the logic for
> the different flag combinations split into their own commits, but
> I am open to any suggestions.

Yeah, there are some alternatives, all valid.  

Support matching/all combination in 1/5, document that in 2/5, test
that in 3/5 and then do the same 3-patch series to support
matching/normal combination in 4/5, 5/5 and 6/5 would be one.  Doing
code, doc and test for matching/all in one patch and doing code, doc
and test for matching/normal in another patch, resulting in a
two-patch series may be another.  Or your "code for matching/all in
1/5, code for matching/normal in 2/5, doc and test for both in
subsequent steps" is fine, too.  All of these would not leave things
in inconsistent state when we interrupt the series in the middle,
which is a desirable property for those who want to understand the
topic.


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

* [PATCH v3 0/4] status: add option to show ignored files differently
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
                   ` (7 preceding siblings ...)
  2017-10-11 13:34 ` [PATCH v2 0/5] " Jameson Miller
@ 2017-10-19 16:05 ` Jameson Miller
  2017-10-19 16:05   ` [PATCH v3 1/4] " Jameson Miller
                     ` (4 more replies)
  2017-10-23 17:05 ` [PATCH v4 " Jameson Miller
  2017-10-30 17:21 ` [PATCH v5 0/4] status: add option to show ignored files differently jameson.miller81
  10 siblings, 5 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-19 16:05 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

The previous iteration can be found here:

https://public-inbox.org/git/20171011133504.15049-1-jamill@microsoft.com/

The main difference is to address feedback around commit organization
and wording.

Jameson Miller (4):
  status: add option to show ignored files differently
  status: report matching ignored and normal untracked
  status: document options to show matching ignored files
  status: test --ignored=matching

 Documentation/git-status.txt                      |  21 ++-
 Documentation/technical/api-directory-listing.txt |  27 +++-
 builtin/commit.c                                  |  31 +++-
 dir.c                                             |  44 +++++-
 dir.h                                             |   3 +-
 t/t7519-ignored-mode.sh                           | 183 ++++++++++++++++++++++
 wt-status.c                                       |  11 +-
 wt-status.h                                       |   8 +-
 8 files changed, 310 insertions(+), 18 deletions(-)
 create mode 100755 t/t7519-ignored-mode.sh

-- 
2.13.6


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

* [PATCH v3 1/4] status: add option to show ignored files differently
  2017-10-19 16:05 ` [PATCH v3 0/4] status: add option to show ignored files differently Jameson Miller
@ 2017-10-19 16:05   ` Jameson Miller
  2017-10-23  3:34     ` Junio C Hamano
  2017-10-19 16:05   ` [PATCH v3 2/4] status: report matching ignored and normal untracked Jameson Miller
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 49+ messages in thread
From: Jameson Miller @ 2017-10-19 16:05 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

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. There are 2 benefits to these changes: 1) if a consumer needs
all untracked files but not all ignored files, there is a performance
benefit to not scanning all contents of an ignored directory and 2)
returning ignored files that explicitly match a path allow a consumer
to make more informed decisions about when a status result might be
stale.

This commit implements --ignored=matching with --untracked-files=all.
The following commit will implement --ignored=matching with
--untracked=files=normal.

As an example of where this flexibility could be useful is that our
application (Visual Studio) runs the status command and presents the
output. It shows all untracked files individually (e.g. using the
'--untracked-files==all' option), and would like to know about which
paths are ignored. It uses information about ignored paths to make
decisions about when the status result might have changed.
Additionally, many projects place build output into directories inside
a repository's working directory (e.g. in "bin/" and "obj/"
directories). Normal usage is to explicitly ignore these 2 directory
names in the .gitignore file (rather than or in addition to the *.obj
pattern).If an application could know that these directories are
explicitly ignored, it could infer that all contents are ignored as
well and make better informed decisions about files in these
directories. It could infer that any changes under these paths would
not affect the output of status. Additionally, there can be a
significant performance benefit by avoiding scanning through ignored
directories.

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.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 builtin/commit.c | 31 +++++++++++++++++++++++++------
 dir.c            | 24 ++++++++++++++++++++++++
 dir.h            |  3 ++-
 wt-status.c      | 11 ++++++++---
 wt-status.h      |  8 +++++++-
 5 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d75b3805ea..98d84d0277 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -118,7 +118,7 @@ static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message;
-static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
+static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit;
 
 /*
@@ -139,7 +139,7 @@ static const char *cleanup_arg;
 static enum commit_whence whence;
 static int sequencer_in_use;
 static int use_editor = 1, include_status = 1;
-static int show_ignored_in_status, have_option_m;
+static int have_option_m;
 static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
@@ -1075,6 +1075,19 @@ static const char *find_author_by_nickname(const char *name)
 	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
 }
 
+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" },
 		{ 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" },
@@ -1383,8 +1398,12 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	finalize_deferred_config(&s);
 
 	handle_untracked_files_arg(&s);
-	if (show_ignored_in_status)
-		s.show_ignored_files = 1;
+	handle_ignored_arg(&s);
+
+	if (s.show_ignored_mode == SHOW_MATCHING_IGNORED &&
+	    s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
+		die(_("Unsupported combination of ignored and untracked-files arguments"));
+
 	parse_pathspec(&s.pathspec, 0,
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
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)) {
+
+			/*
+			 * 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).
+			 */
+			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;
+		}
 		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;
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) {
 		dir.flags |= DIR_SHOW_IGNORED_TOO;
-	else
+
+		if (s->show_ignored_mode == SHOW_MATCHING_IGNORED)
+			dir.flags |= DIR_SHOW_IGNORED_TOO_MODE_MATCHING;
+	} 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)
 			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", "");
diff --git a/wt-status.h b/wt-status.h
index 64f4d33ea1..fe27b465e2 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -27,6 +27,12 @@ enum untracked_status_type {
 	SHOW_ALL_UNTRACKED_FILES
 };
 
+enum show_ignored_type {
+	SHOW_NO_IGNORED,
+	SHOW_TRADITIONAL_IGNORED,
+	SHOW_MATCHING_IGNORED,
+};
+
 /* from where does this commit originate */
 enum commit_whence {
 	FROM_COMMIT,     /* normal */
@@ -70,7 +76,7 @@ struct wt_status {
 	int display_comment_prefix;
 	int relative_paths;
 	int submodule_summary;
-	int show_ignored_files;
+	enum show_ignored_type show_ignored_mode;
 	enum untracked_status_type show_untracked_files;
 	const char *ignore_submodule_arg;
 	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];
-- 
2.13.6


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

* [PATCH v3 2/4] status: report matching ignored and normal untracked
  2017-10-19 16:05 ` [PATCH v3 0/4] status: add option to show ignored files differently Jameson Miller
  2017-10-19 16:05   ` [PATCH v3 1/4] " Jameson Miller
@ 2017-10-19 16:05   ` Jameson Miller
  2017-10-19 16:06   ` [PATCH v3 3/4] status: document options to show matching ignored files Jameson Miller
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-19 16:05 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Teach status command to handle `--ignored=matching` with
`--untracked-files=normal`

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 dir.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index b9af87eca9..20457724c0 100644
--- a/dir.c
+++ b/dir.c
@@ -1585,6 +1585,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 {
 	int exclude;
 	int has_path_in_index = !!index_file_exists(istate, path->buf, path->len, ignore_case);
+	enum path_treatment path_treatment;
 
 	if (dtype == DT_UNKNOWN)
 		dtype = get_dtype(de, istate, path->buf, path->len);
@@ -1631,8 +1632,23 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 		return path_none;
 	case DT_DIR:
 		strbuf_addch(path, '/');
-		return treat_directory(dir, istate, untracked, path->buf, path->len,
-				       baselen, exclude, pathspec);
+		path_treatment = treat_directory(dir, istate, untracked,
+						 path->buf, path->len,
+						 baselen, exclude, pathspec);
+		/*
+		 * If 1) we only want to return directories that
+		 * match an exclude pattern and 2) this directory does
+		 * not match an exclude pattern but all of its
+		 * contents are excluded, then indicate that we should
+		 * recurse into this directory (instead of marking the
+		 * directory itself as an ignored path).
+		 */
+		if (!exclude &&
+		    path_treatment == path_excluded &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO) &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO_MODE_MATCHING))
+			return path_recurse;
+		return path_treatment;
 	case DT_REG:
 	case DT_LNK:
 		return exclude ? path_excluded : path_untracked;
-- 
2.13.6


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

* [PATCH v3 3/4] status: document options to show matching ignored files
  2017-10-19 16:05 ` [PATCH v3 0/4] status: add option to show ignored files differently Jameson Miller
  2017-10-19 16:05   ` [PATCH v3 1/4] " Jameson Miller
  2017-10-19 16:05   ` [PATCH v3 2/4] status: report matching ignored and normal untracked Jameson Miller
@ 2017-10-19 16:06   ` Jameson Miller
  2017-10-19 16:06   ` [PATCH v3 4/4] status: test --ignored=matching Jameson Miller
  2017-10-23  4:46   ` [PATCH v3 0/4] status: add option to show ignored files differently Junio C Hamano
  4 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-19 16:06 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 Documentation/git-status.txt                      | 21 +++++++++++++++++-
 Documentation/technical/api-directory-listing.txt | 27 +++++++++++++++++++----
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 9f3a78a36c..fc282e0a92 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -97,8 +97,27 @@ configuration variable documented in linkgit:git-config[1].
 	(and suppresses the output of submodule summaries when the config option
 	`status.submoduleSummary` is set).
 
---ignored::
+--ignored[=<mode>]::
 	Show ignored files as well.
++
+The mode parameter is used to specify the handling of ignored files.
+It is optional: it defaults to 'traditional'.
++
+The possible options are:
++
+	- 'traditional' - Shows ignored files and directories, unless
+			  --untracked-files=all is specifed, in which case
+			  individual files in ignored directories are
+			  displayed.
+	- 'no'	        - Show no ignored files.
+	- 'matching'    - Shows ignored files and directories matching an
+			  ignore pattern.
++
+When 'matching' mode is specified, paths that explicity match an
+ignored pattern are shown. If a directory matches an ignore pattern,
+then it is shown, but not paths contained in the ignored directory. If
+a directory does not match an ignore pattern, but all contents are
+ignored, then the directory is not shown, but all contents are shown.
 
 -z::
 	Terminate entries with NUL, instead of LF.  This implies
diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
index 6c77b4920c..7fae00f44f 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -22,16 +22,20 @@ The notable options are:
 
 `flags`::
 
-	A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
+	A bit-field of options:
 
 `DIR_SHOW_IGNORED`:::
 
-	Return just ignored files in `entries[]`, not untracked files.
+	Return just ignored files in `entries[]`, not untracked
+	files. This flag is mutually exclusive with
+	`DIR_SHOW_IGNORED_TOO`.
 
 `DIR_SHOW_IGNORED_TOO`:::
 
-	Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
-	in addition to untracked files in `entries[]`.
+	Similar to `DIR_SHOW_IGNORED`, but return ignored files in
+	`ignored[]` in addition to untracked files in
+	`entries[]`. This flag is mutually exclusive with
+	`DIR_SHOW_IGNORED`.
 
 `DIR_KEEP_UNTRACKED_CONTENTS`:::
 
@@ -39,6 +43,21 @@ The notable options are:
 	untracked contents of untracked directories are also returned in
 	`entries[]`.
 
+`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
+
+	Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
+	this is set, returns ignored files and directories that match
+	an exclude pattern. If a directory matches an exclude pattern,
+	then the directory is returned and the contained paths are
+	not. A directory that does not match an exclude pattern will
+	not be returned even if all of its contents are ignored. In
+	this case, the contents are returned as individual entries.
++
+If this is set, files and directories that explicity match an ignore
+pattern are reported. Implicity ignored directories (directories that
+do not match an ignore pattern, but whose contents are all ignored)
+are not reported, instead all of the contents are reported.
+
 `DIR_COLLECT_IGNORED`:::
 
 	Special mode for git-add. Return ignored files in `ignored[]` and
-- 
2.13.6


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

* [PATCH v3 4/4] status: test --ignored=matching
  2017-10-19 16:05 ` [PATCH v3 0/4] status: add option to show ignored files differently Jameson Miller
                     ` (2 preceding siblings ...)
  2017-10-19 16:06   ` [PATCH v3 3/4] status: document options to show matching ignored files Jameson Miller
@ 2017-10-19 16:06   ` Jameson Miller
  2017-10-19 19:33     ` Stefan Beller
  2017-10-22  2:10     ` Junio C Hamano
  2017-10-23  4:46   ` [PATCH v3 0/4] status: add option to show ignored files differently Junio C Hamano
  4 siblings, 2 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-19 16:06 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Add tests around status reporting ignord files that match an exclude
pattern for both --untracked-files=normal and --untracked-files=all.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 t/t7519-ignored-mode.sh | 183 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 183 insertions(+)
 create mode 100755 t/t7519-ignored-mode.sh

diff --git a/t/t7519-ignored-mode.sh b/t/t7519-ignored-mode.sh
new file mode 100755
index 0000000000..6be7701d79
--- /dev/null
+++ b/t/t7519-ignored-mode.sh
@@ -0,0 +1,183 @@
+#!/bin/sh
+
+test_description='git status collapse ignored'
+
+. ./test-lib.sh
+
+# commit initial ignore file
+test_expect_success 'setup initial commit and ignore file' '
+	cat >.gitignore <<-\EOF &&
+	*.ign
+	ignored_dir/
+	!*.unignore
+	EOF
+	git add . &&
+	git commit -m "Initial commit"
+'
+
+test_expect_success 'Verify behavior of status on folders with ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/ignored/ignored_1.ign
+	! dir/ignored/ignored_2.ign
+	! ignored/ignored_1.ign
+	! ignored/ignored_2.ign
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on folder with tracked & ignored files' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/tracked_ignored/ignored_1.ign
+	! dir/tracked_ignored/ignored_2.ign
+	! tracked_ignored/ignored_1.ign
+	! tracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p tracked_ignored dir/tracked_ignored &&
+	touch tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		tracked_ignored/ignored_1.ign tracked_ignored/ignored_2.ign \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 \
+		dir/tracked_ignored/ignored_1.ign dir/tracked_ignored/ignored_2.ign &&
+
+	git add tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 &&
+	git commit -m "commit tracked files" &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on folder with untracked and ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? dir/untracked_ignored/untracked_1
+	? dir/untracked_ignored/untracked_2
+	? expect
+	? output
+	? untracked_ignored/untracked_1
+	? untracked_ignored/untracked_2
+	! dir/untracked_ignored/ignored_1.ign
+	! dir/untracked_ignored/ignored_2.ign
+	! untracked_ignored/ignored_1.ign
+	! untracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p untracked_ignored dir/untracked_ignored &&
+	touch untracked_ignored/untracked_1 untracked_ignored/untracked_2 \
+		untracked_ignored/ignored_1.ign untracked_ignored/ignored_2.ign \
+		dir/untracked_ignored/untracked_1 dir/untracked_ignored/untracked_2 \
+		dir/untracked_ignored/ignored_1.ign dir/untracked_ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status matching ignored files on ignored folder' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored folder containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored folder containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_done
-- 
2.13.6


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

* Re: [PATCH v3 4/4] status: test --ignored=matching
  2017-10-19 16:06   ` [PATCH v3 4/4] status: test --ignored=matching Jameson Miller
@ 2017-10-19 19:33     ` Stefan Beller
  2017-10-22  2:10     ` Junio C Hamano
  1 sibling, 0 replies; 49+ messages in thread
From: Stefan Beller @ 2017-10-19 19:33 UTC (permalink / raw)
  To: Jameson Miller
  Cc: Brandon Williams, git@vger.kernel.org, Junio C Hamano,
	Jameson Miller, Jeff King

On Thu, Oct 19, 2017 at 9:06 AM, Jameson Miller
<jameson.miller81@gmail.com> wrote:

> +test_expect_success 'Verify behavior of status on folders with ignored files' '

https://stackoverflow.com/questions/5078676/what-is-the-difference-between-a-directory-and-a-folder
We test directories here?

All tests are testing --ignored=matching, do we want to have at least one test
of "no" and "traditional" as well? (Just to see if the parsing works;
we can modify
an existing test for that?)

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

* Re: [PATCH v3 4/4] status: test --ignored=matching
  2017-10-19 16:06   ` [PATCH v3 4/4] status: test --ignored=matching Jameson Miller
  2017-10-19 19:33     ` Stefan Beller
@ 2017-10-22  2:10     ` Junio C Hamano
  1 sibling, 0 replies; 49+ messages in thread
From: Junio C Hamano @ 2017-10-22  2:10 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

> Add tests around status reporting ignord files that match an exclude
> pattern for both --untracked-files=normal and --untracked-files=all.
>
> Signed-off-by: Jameson Miller <jamill@microsoft.com>
> ---
>  t/t7519-ignored-mode.sh | 183 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 183 insertions(+)
>  create mode 100755 t/t7519-ignored-mode.sh

Ben's fsmonitor series already uses t7519, so please move this
somewhere else.  Perhaps

    git diff --name-only --diff-filter=A master pu t/

would help you see what new tests that are missing from 'master'
are added by topics in flight.

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

* Re: [PATCH v3 1/4] status: add option to show ignored files differently
  2017-10-19 16:05   ` [PATCH v3 1/4] " Jameson Miller
@ 2017-10-23  3:34     ` Junio C Hamano
  0 siblings, 0 replies; 49+ messages in thread
From: Junio C Hamano @ 2017-10-23  3:34 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

> 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.

> +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" },

> 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) {

>  		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.

> +	} 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", "");

> 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.

> +			/*
> +			 * 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"?

> +			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.

>  		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;

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

* Re: [PATCH v3 0/4] status: add option to show ignored files differently
  2017-10-19 16:05 ` [PATCH v3 0/4] status: add option to show ignored files differently Jameson Miller
                     ` (3 preceding siblings ...)
  2017-10-19 16:06   ` [PATCH v3 4/4] status: test --ignored=matching Jameson Miller
@ 2017-10-23  4:46   ` Junio C Hamano
  4 siblings, 0 replies; 49+ messages in thread
From: Junio C Hamano @ 2017-10-23  4:46 UTC (permalink / raw)
  To: Jameson Miller; +Cc: bmwill, git, jamill, peff, sbeller

Jameson Miller <jameson.miller81@gmail.com> writes:

> Jameson Miller (4):
>   status: add option to show ignored files differently
>   status: report matching ignored and normal untracked
>   status: document options to show matching ignored files
>   status: test --ignored=matching

Thanks, queued.

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

* [PATCH v4 0/4] status: add option to show ignored files differently
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
                   ` (8 preceding siblings ...)
  2017-10-19 16:05 ` [PATCH v3 0/4] status: add option to show ignored files differently Jameson Miller
@ 2017-10-23 17:05 ` Jameson Miller
  2017-10-23 17:05   ` [PATCH v4 1/4] " Jameson Miller
                     ` (3 more replies)
  2017-10-30 17:21 ` [PATCH v5 0/4] status: add option to show ignored files differently jameson.miller81
  10 siblings, 4 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-23 17:05 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

The previous iteration can be found here:

https://public-inbox.org/git/20171019160601.9382-1-jamill@microsoft.com/


Changes from V3 patch series:
 - Added extra test for --ignored=no an --ignored=traditional
 - Updated wording from "folders" -> "directories"
 - Renamed test to a name not taken in by topics in flight

Jameson Miller (4):
  status: add option to show ignored files differently
  status: report matching ignored and normal untracked
  status: document options to show matching ignored files
  status: test ignored modes

 Documentation/git-status.txt                      |  21 +-
 Documentation/technical/api-directory-listing.txt |  27 ++-
 builtin/commit.c                                  |  31 ++-
 dir.c                                             |  44 +++-
 dir.h                                             |   3 +-
 t/t7521-ignored-mode.sh                           | 233 ++++++++++++++++++++++
 wt-status.c                                       |  11 +-
 wt-status.h                                       |   8 +-
 8 files changed, 360 insertions(+), 18 deletions(-)
 create mode 100755 t/t7521-ignored-mode.sh

-- 
2.13.6


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

* [PATCH v4 1/4] status: add option to show ignored files differently
  2017-10-23 17:05 ` [PATCH v4 " Jameson Miller
@ 2017-10-23 17:05   ` Jameson Miller
  2017-10-23 17:05   ` [PATCH v4 2/4] status: report matching ignored and normal untracked Jameson Miller
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-23 17:05 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

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. There are 2 benefits to these changes: 1) if a consumer needs
all untracked files but not all ignored files, there is a performance
benefit to not scanning all contents of an ignored directory and 2)
returning ignored files that explicitly match a path allow a consumer
to make more informed decisions about when a status result might be
stale.

This commit implements --ignored=matching with --untracked-files=all.
The following commit will implement --ignored=matching with
--untracked=files=normal.

As an example of where this flexibility could be useful is that our
application (Visual Studio) runs the status command and presents the
output. It shows all untracked files individually (e.g. using the
'--untracked-files==all' option), and would like to know about which
paths are ignored. It uses information about ignored paths to make
decisions about when the status result might have changed.
Additionally, many projects place build output into directories inside
a repository's working directory (e.g. in "bin/" and "obj/"
directories). Normal usage is to explicitly ignore these 2 directory
names in the .gitignore file (rather than or in addition to the *.obj
pattern).If an application could know that these directories are
explicitly ignored, it could infer that all contents are ignored as
well and make better informed decisions about files in these
directories. It could infer that any changes under these paths would
not affect the output of status. Additionally, there can be a
significant performance benefit by avoiding scanning through ignored
directories.

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.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 builtin/commit.c | 31 +++++++++++++++++++++++++------
 dir.c            | 24 ++++++++++++++++++++++++
 dir.h            |  3 ++-
 wt-status.c      | 11 ++++++++---
 wt-status.h      |  8 +++++++-
 5 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d75b3805ea..98d84d0277 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -118,7 +118,7 @@ static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message;
-static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
+static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit;
 
 /*
@@ -139,7 +139,7 @@ static const char *cleanup_arg;
 static enum commit_whence whence;
 static int sequencer_in_use;
 static int use_editor = 1, include_status = 1;
-static int show_ignored_in_status, have_option_m;
+static int have_option_m;
 static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
@@ -1075,6 +1075,19 @@ static const char *find_author_by_nickname(const char *name)
 	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
 }
 
+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" },
 		{ 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" },
@@ -1383,8 +1398,12 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	finalize_deferred_config(&s);
 
 	handle_untracked_files_arg(&s);
-	if (show_ignored_in_status)
-		s.show_ignored_files = 1;
+	handle_ignored_arg(&s);
+
+	if (s.show_ignored_mode == SHOW_MATCHING_IGNORED &&
+	    s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
+		die(_("Unsupported combination of ignored and untracked-files arguments"));
+
 	parse_pathspec(&s.pathspec, 0,
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
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)) {
+
+			/*
+			 * 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).
+			 */
+			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;
+		}
 		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;
diff --git a/wt-status.c b/wt-status.c
index 29bc64cc02..6760df6176 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -658,10 +658,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) {
 		dir.flags |= DIR_SHOW_IGNORED_TOO;
-	else
+
+		if (s->show_ignored_mode == SHOW_MATCHING_IGNORED)
+			dir.flags |= DIR_SHOW_IGNORED_TOO_MODE_MATCHING;
+	} else {
 		dir.untracked = the_index.untracked;
+	}
+
 	setup_standard_excludes(&dir);
 
 	fill_directory(&dir, &the_index, &s->pathspec);
@@ -1619,7 +1624,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)
 			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", "");
diff --git a/wt-status.h b/wt-status.h
index 64f4d33ea1..fe27b465e2 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -27,6 +27,12 @@ enum untracked_status_type {
 	SHOW_ALL_UNTRACKED_FILES
 };
 
+enum show_ignored_type {
+	SHOW_NO_IGNORED,
+	SHOW_TRADITIONAL_IGNORED,
+	SHOW_MATCHING_IGNORED,
+};
+
 /* from where does this commit originate */
 enum commit_whence {
 	FROM_COMMIT,     /* normal */
@@ -70,7 +76,7 @@ struct wt_status {
 	int display_comment_prefix;
 	int relative_paths;
 	int submodule_summary;
-	int show_ignored_files;
+	enum show_ignored_type show_ignored_mode;
 	enum untracked_status_type show_untracked_files;
 	const char *ignore_submodule_arg;
 	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];
-- 
2.13.6


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

* [PATCH v4 2/4] status: report matching ignored and normal untracked
  2017-10-23 17:05 ` [PATCH v4 " Jameson Miller
  2017-10-23 17:05   ` [PATCH v4 1/4] " Jameson Miller
@ 2017-10-23 17:05   ` Jameson Miller
  2017-10-23 17:05   ` [PATCH v4 3/4] status: document options to show matching ignored files Jameson Miller
  2017-10-23 17:05   ` [PATCH v4 4/4] status: test ignored modes Jameson Miller
  3 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-23 17:05 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Teach status command to handle `--ignored=matching` with
`--untracked-files=normal`

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 dir.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index b9af87eca9..20457724c0 100644
--- a/dir.c
+++ b/dir.c
@@ -1585,6 +1585,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 {
 	int exclude;
 	int has_path_in_index = !!index_file_exists(istate, path->buf, path->len, ignore_case);
+	enum path_treatment path_treatment;
 
 	if (dtype == DT_UNKNOWN)
 		dtype = get_dtype(de, istate, path->buf, path->len);
@@ -1631,8 +1632,23 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 		return path_none;
 	case DT_DIR:
 		strbuf_addch(path, '/');
-		return treat_directory(dir, istate, untracked, path->buf, path->len,
-				       baselen, exclude, pathspec);
+		path_treatment = treat_directory(dir, istate, untracked,
+						 path->buf, path->len,
+						 baselen, exclude, pathspec);
+		/*
+		 * If 1) we only want to return directories that
+		 * match an exclude pattern and 2) this directory does
+		 * not match an exclude pattern but all of its
+		 * contents are excluded, then indicate that we should
+		 * recurse into this directory (instead of marking the
+		 * directory itself as an ignored path).
+		 */
+		if (!exclude &&
+		    path_treatment == path_excluded &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO) &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO_MODE_MATCHING))
+			return path_recurse;
+		return path_treatment;
 	case DT_REG:
 	case DT_LNK:
 		return exclude ? path_excluded : path_untracked;
-- 
2.13.6


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

* [PATCH v4 3/4] status: document options to show matching ignored files
  2017-10-23 17:05 ` [PATCH v4 " Jameson Miller
  2017-10-23 17:05   ` [PATCH v4 1/4] " Jameson Miller
  2017-10-23 17:05   ` [PATCH v4 2/4] status: report matching ignored and normal untracked Jameson Miller
@ 2017-10-23 17:05   ` Jameson Miller
  2017-10-23 17:05   ` [PATCH v4 4/4] status: test ignored modes Jameson Miller
  3 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-23 17:05 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 Documentation/git-status.txt                      | 21 +++++++++++++++++-
 Documentation/technical/api-directory-listing.txt | 27 +++++++++++++++++++----
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 9f3a78a36c..fc282e0a92 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -97,8 +97,27 @@ configuration variable documented in linkgit:git-config[1].
 	(and suppresses the output of submodule summaries when the config option
 	`status.submoduleSummary` is set).
 
---ignored::
+--ignored[=<mode>]::
 	Show ignored files as well.
++
+The mode parameter is used to specify the handling of ignored files.
+It is optional: it defaults to 'traditional'.
++
+The possible options are:
++
+	- 'traditional' - Shows ignored files and directories, unless
+			  --untracked-files=all is specifed, in which case
+			  individual files in ignored directories are
+			  displayed.
+	- 'no'	        - Show no ignored files.
+	- 'matching'    - Shows ignored files and directories matching an
+			  ignore pattern.
++
+When 'matching' mode is specified, paths that explicity match an
+ignored pattern are shown. If a directory matches an ignore pattern,
+then it is shown, but not paths contained in the ignored directory. If
+a directory does not match an ignore pattern, but all contents are
+ignored, then the directory is not shown, but all contents are shown.
 
 -z::
 	Terminate entries with NUL, instead of LF.  This implies
diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
index 6c77b4920c..7fae00f44f 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -22,16 +22,20 @@ The notable options are:
 
 `flags`::
 
-	A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
+	A bit-field of options:
 
 `DIR_SHOW_IGNORED`:::
 
-	Return just ignored files in `entries[]`, not untracked files.
+	Return just ignored files in `entries[]`, not untracked
+	files. This flag is mutually exclusive with
+	`DIR_SHOW_IGNORED_TOO`.
 
 `DIR_SHOW_IGNORED_TOO`:::
 
-	Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
-	in addition to untracked files in `entries[]`.
+	Similar to `DIR_SHOW_IGNORED`, but return ignored files in
+	`ignored[]` in addition to untracked files in
+	`entries[]`. This flag is mutually exclusive with
+	`DIR_SHOW_IGNORED`.
 
 `DIR_KEEP_UNTRACKED_CONTENTS`:::
 
@@ -39,6 +43,21 @@ The notable options are:
 	untracked contents of untracked directories are also returned in
 	`entries[]`.
 
+`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
+
+	Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
+	this is set, returns ignored files and directories that match
+	an exclude pattern. If a directory matches an exclude pattern,
+	then the directory is returned and the contained paths are
+	not. A directory that does not match an exclude pattern will
+	not be returned even if all of its contents are ignored. In
+	this case, the contents are returned as individual entries.
++
+If this is set, files and directories that explicity match an ignore
+pattern are reported. Implicity ignored directories (directories that
+do not match an ignore pattern, but whose contents are all ignored)
+are not reported, instead all of the contents are reported.
+
 `DIR_COLLECT_IGNORED`:::
 
 	Special mode for git-add. Return ignored files in `ignored[]` and
-- 
2.13.6


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

* [PATCH v4 4/4] status: test ignored modes
  2017-10-23 17:05 ` [PATCH v4 " Jameson Miller
                     ` (2 preceding siblings ...)
  2017-10-23 17:05   ` [PATCH v4 3/4] status: document options to show matching ignored files Jameson Miller
@ 2017-10-23 17:05   ` Jameson Miller
  3 siblings, 0 replies; 49+ messages in thread
From: Jameson Miller @ 2017-10-23 17:05 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

Add tests around status reporting ignord files that match an exclude
pattern for both --untracked-files=normal and --untracked-files=all.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 t/t7521-ignored-mode.sh | 233 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 233 insertions(+)
 create mode 100755 t/t7521-ignored-mode.sh

diff --git a/t/t7521-ignored-mode.sh b/t/t7521-ignored-mode.sh
new file mode 100755
index 0000000000..91790943c3
--- /dev/null
+++ b/t/t7521-ignored-mode.sh
@@ -0,0 +1,233 @@
+#!/bin/sh
+
+test_description='git status ignored modes'
+
+. ./test-lib.sh
+
+test_expect_success 'setup initial commit and ignore file' '
+	cat >.gitignore <<-\EOF &&
+	*.ign
+	ignored_dir/
+	!*.unignore
+	EOF
+	git add . &&
+	git commit -m "Initial commit"
+'
+
+test_expect_success 'Verify behavior of status on directories with ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/ignored/ignored_1.ign
+	! dir/ignored/ignored_2.ign
+	! ignored/ignored_1.ign
+	! ignored/ignored_2.ign
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on directory with tracked & ignored files' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/tracked_ignored/ignored_1.ign
+	! dir/tracked_ignored/ignored_2.ign
+	! tracked_ignored/ignored_1.ign
+	! tracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p tracked_ignored dir/tracked_ignored &&
+	touch tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		tracked_ignored/ignored_1.ign tracked_ignored/ignored_2.ign \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 \
+		dir/tracked_ignored/ignored_1.ign dir/tracked_ignored/ignored_2.ign &&
+
+	git add tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 &&
+	git commit -m "commit tracked files" &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on directory with untracked and ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? dir/untracked_ignored/untracked_1
+	? dir/untracked_ignored/untracked_2
+	? expect
+	? output
+	? untracked_ignored/untracked_1
+	? untracked_ignored/untracked_2
+	! dir/untracked_ignored/ignored_1.ign
+	! dir/untracked_ignored/ignored_2.ign
+	! untracked_ignored/ignored_1.ign
+	! untracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p untracked_ignored dir/untracked_ignored &&
+	touch untracked_ignored/untracked_1 untracked_ignored/untracked_2 \
+		untracked_ignored/ignored_1.ign untracked_ignored/ignored_2.ign \
+		dir/untracked_ignored/untracked_1 dir/untracked_ignored/untracked_2 \
+		dir/untracked_ignored/ignored_1.ign dir/untracked_ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status matching ignored files on ignored directory' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored directory containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored directory containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify behavior of status with --ignored=no' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=no --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify behavior of status with --ignored=traditional and --untracked-files=all' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/ignored/ignored_1.ign
+	! dir/ignored/ignored_2.ign
+	! ignored/ignored_1.ign
+	! ignored/ignored_2.ign
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=traditional --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify behavior of status with --ignored=traditional and --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/
+	! ignored/
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=traditional --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_done
-- 
2.13.6


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

* [PATCH v5 0/4] status: add option to show ignored files differently
  2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
                   ` (9 preceding siblings ...)
  2017-10-23 17:05 ` [PATCH v4 " Jameson Miller
@ 2017-10-30 17:21 ` jameson.miller81
  2017-10-30 17:21   ` [PATCH v5 1/4] " jameson.miller81
                     ` (4 more replies)
  10 siblings, 5 replies; 49+ messages in thread
From: jameson.miller81 @ 2017-10-30 17:21 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

From: Jameson Miller <jamill@microsoft.com>

Previous patch series can be found:
https://public-inbox.org/git/20171023170534.157740-1-jamill@microsoft.com/

Only difference from previous version is to update the commit author
email to match corporate email address.

Jameson Miller (4):
  status: add option to show ignored files differently
  status: report matching ignored and normal untracked
  status: document options to show matching ignored files
  status: test ignored modes

 Documentation/git-status.txt                      |  21 +-
 Documentation/technical/api-directory-listing.txt |  27 ++-
 builtin/commit.c                                  |  31 ++-
 dir.c                                             |  44 +++-
 dir.h                                             |   3 +-
 t/t7521-ignored-mode.sh                           | 233 ++++++++++++++++++++++
 wt-status.c                                       |  11 +-
 wt-status.h                                       |   8 +-
 8 files changed, 360 insertions(+), 18 deletions(-)
 create mode 100755 t/t7521-ignored-mode.sh

-- 
2.13.6


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

* [PATCH v5 1/4] status: add option to show ignored files differently
  2017-10-30 17:21 ` [PATCH v5 0/4] status: add option to show ignored files differently jameson.miller81
@ 2017-10-30 17:21   ` jameson.miller81
  2017-10-30 17:21   ` [PATCH v5 2/4] status: report matching ignored and normal untracked jameson.miller81
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 49+ messages in thread
From: jameson.miller81 @ 2017-10-30 17:21 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

From: Jameson Miller <jamill@microsoft.com>

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. There are 2 benefits to these changes: 1) if a consumer needs
all untracked files but not all ignored files, there is a performance
benefit to not scanning all contents of an ignored directory and 2)
returning ignored files that explicitly match a path allow a consumer
to make more informed decisions about when a status result might be
stale.

This commit implements --ignored=matching with --untracked-files=all.
The following commit will implement --ignored=matching with
--untracked=files=normal.

As an example of where this flexibility could be useful is that our
application (Visual Studio) runs the status command and presents the
output. It shows all untracked files individually (e.g. using the
'--untracked-files==all' option), and would like to know about which
paths are ignored. It uses information about ignored paths to make
decisions about when the status result might have changed.
Additionally, many projects place build output into directories inside
a repository's working directory (e.g. in "bin/" and "obj/"
directories). Normal usage is to explicitly ignore these 2 directory
names in the .gitignore file (rather than or in addition to the *.obj
pattern).If an application could know that these directories are
explicitly ignored, it could infer that all contents are ignored as
well and make better informed decisions about files in these
directories. It could infer that any changes under these paths would
not affect the output of status. Additionally, there can be a
significant performance benefit by avoiding scanning through ignored
directories.

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.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 builtin/commit.c | 31 +++++++++++++++++++++++++------
 dir.c            | 24 ++++++++++++++++++++++++
 dir.h            |  3 ++-
 wt-status.c      | 11 ++++++++---
 wt-status.h      |  8 +++++++-
 5 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d75b3805ea..98d84d0277 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -118,7 +118,7 @@ static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message;
-static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
+static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit;
 
 /*
@@ -139,7 +139,7 @@ static const char *cleanup_arg;
 static enum commit_whence whence;
 static int sequencer_in_use;
 static int use_editor = 1, include_status = 1;
-static int show_ignored_in_status, have_option_m;
+static int have_option_m;
 static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
@@ -1075,6 +1075,19 @@ static const char *find_author_by_nickname(const char *name)
 	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
 }
 
+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" },
 		{ 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" },
@@ -1383,8 +1398,12 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	finalize_deferred_config(&s);
 
 	handle_untracked_files_arg(&s);
-	if (show_ignored_in_status)
-		s.show_ignored_files = 1;
+	handle_ignored_arg(&s);
+
+	if (s.show_ignored_mode == SHOW_MATCHING_IGNORED &&
+	    s.show_untracked_files == SHOW_NO_UNTRACKED_FILES)
+		die(_("Unsupported combination of ignored and untracked-files arguments"));
+
 	parse_pathspec(&s.pathspec, 0,
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
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)) {
+
+			/*
+			 * 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).
+			 */
+			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;
+		}
 		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;
diff --git a/wt-status.c b/wt-status.c
index 29bc64cc02..6760df6176 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -658,10 +658,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) {
 		dir.flags |= DIR_SHOW_IGNORED_TOO;
-	else
+
+		if (s->show_ignored_mode == SHOW_MATCHING_IGNORED)
+			dir.flags |= DIR_SHOW_IGNORED_TOO_MODE_MATCHING;
+	} else {
 		dir.untracked = the_index.untracked;
+	}
+
 	setup_standard_excludes(&dir);
 
 	fill_directory(&dir, &the_index, &s->pathspec);
@@ -1619,7 +1624,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)
 			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", "");
diff --git a/wt-status.h b/wt-status.h
index 64f4d33ea1..fe27b465e2 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -27,6 +27,12 @@ enum untracked_status_type {
 	SHOW_ALL_UNTRACKED_FILES
 };
 
+enum show_ignored_type {
+	SHOW_NO_IGNORED,
+	SHOW_TRADITIONAL_IGNORED,
+	SHOW_MATCHING_IGNORED,
+};
+
 /* from where does this commit originate */
 enum commit_whence {
 	FROM_COMMIT,     /* normal */
@@ -70,7 +76,7 @@ struct wt_status {
 	int display_comment_prefix;
 	int relative_paths;
 	int submodule_summary;
-	int show_ignored_files;
+	enum show_ignored_type show_ignored_mode;
 	enum untracked_status_type show_untracked_files;
 	const char *ignore_submodule_arg;
 	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];
-- 
2.13.6


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

* [PATCH v5 2/4] status: report matching ignored and normal untracked
  2017-10-30 17:21 ` [PATCH v5 0/4] status: add option to show ignored files differently jameson.miller81
  2017-10-30 17:21   ` [PATCH v5 1/4] " jameson.miller81
@ 2017-10-30 17:21   ` jameson.miller81
  2017-10-30 17:21   ` [PATCH v5 3/4] status: document options to show matching ignored files jameson.miller81
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 49+ messages in thread
From: jameson.miller81 @ 2017-10-30 17:21 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

From: Jameson Miller <jamill@microsoft.com>

Teach status command to handle `--ignored=matching` with
`--untracked-files=normal`

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 dir.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index b9af87eca9..20457724c0 100644
--- a/dir.c
+++ b/dir.c
@@ -1585,6 +1585,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 {
 	int exclude;
 	int has_path_in_index = !!index_file_exists(istate, path->buf, path->len, ignore_case);
+	enum path_treatment path_treatment;
 
 	if (dtype == DT_UNKNOWN)
 		dtype = get_dtype(de, istate, path->buf, path->len);
@@ -1631,8 +1632,23 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
 		return path_none;
 	case DT_DIR:
 		strbuf_addch(path, '/');
-		return treat_directory(dir, istate, untracked, path->buf, path->len,
-				       baselen, exclude, pathspec);
+		path_treatment = treat_directory(dir, istate, untracked,
+						 path->buf, path->len,
+						 baselen, exclude, pathspec);
+		/*
+		 * If 1) we only want to return directories that
+		 * match an exclude pattern and 2) this directory does
+		 * not match an exclude pattern but all of its
+		 * contents are excluded, then indicate that we should
+		 * recurse into this directory (instead of marking the
+		 * directory itself as an ignored path).
+		 */
+		if (!exclude &&
+		    path_treatment == path_excluded &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO) &&
+		    (dir->flags & DIR_SHOW_IGNORED_TOO_MODE_MATCHING))
+			return path_recurse;
+		return path_treatment;
 	case DT_REG:
 	case DT_LNK:
 		return exclude ? path_excluded : path_untracked;
-- 
2.13.6


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

* [PATCH v5 3/4] status: document options to show matching ignored files
  2017-10-30 17:21 ` [PATCH v5 0/4] status: add option to show ignored files differently jameson.miller81
  2017-10-30 17:21   ` [PATCH v5 1/4] " jameson.miller81
  2017-10-30 17:21   ` [PATCH v5 2/4] status: report matching ignored and normal untracked jameson.miller81
@ 2017-10-30 17:21   ` jameson.miller81
  2017-10-30 17:21   ` [PATCH v5 4/4] status: test ignored modes jameson.miller81
  2017-10-31  3:16   ` [PATCH v5 0/4] status: add option to show ignored files differently Junio C Hamano
  4 siblings, 0 replies; 49+ messages in thread
From: jameson.miller81 @ 2017-10-30 17:21 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

From: Jameson Miller <jamill@microsoft.com>

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 Documentation/git-status.txt                      | 21 +++++++++++++++++-
 Documentation/technical/api-directory-listing.txt | 27 +++++++++++++++++++----
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 9f3a78a36c..fc282e0a92 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -97,8 +97,27 @@ configuration variable documented in linkgit:git-config[1].
 	(and suppresses the output of submodule summaries when the config option
 	`status.submoduleSummary` is set).
 
---ignored::
+--ignored[=<mode>]::
 	Show ignored files as well.
++
+The mode parameter is used to specify the handling of ignored files.
+It is optional: it defaults to 'traditional'.
++
+The possible options are:
++
+	- 'traditional' - Shows ignored files and directories, unless
+			  --untracked-files=all is specifed, in which case
+			  individual files in ignored directories are
+			  displayed.
+	- 'no'	        - Show no ignored files.
+	- 'matching'    - Shows ignored files and directories matching an
+			  ignore pattern.
++
+When 'matching' mode is specified, paths that explicity match an
+ignored pattern are shown. If a directory matches an ignore pattern,
+then it is shown, but not paths contained in the ignored directory. If
+a directory does not match an ignore pattern, but all contents are
+ignored, then the directory is not shown, but all contents are shown.
 
 -z::
 	Terminate entries with NUL, instead of LF.  This implies
diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
index 6c77b4920c..7fae00f44f 100644
--- a/Documentation/technical/api-directory-listing.txt
+++ b/Documentation/technical/api-directory-listing.txt
@@ -22,16 +22,20 @@ The notable options are:
 
 `flags`::
 
-	A bit-field of options (the `*IGNORED*` flags are mutually exclusive):
+	A bit-field of options:
 
 `DIR_SHOW_IGNORED`:::
 
-	Return just ignored files in `entries[]`, not untracked files.
+	Return just ignored files in `entries[]`, not untracked
+	files. This flag is mutually exclusive with
+	`DIR_SHOW_IGNORED_TOO`.
 
 `DIR_SHOW_IGNORED_TOO`:::
 
-	Similar to `DIR_SHOW_IGNORED`, but return ignored files in `ignored[]`
-	in addition to untracked files in `entries[]`.
+	Similar to `DIR_SHOW_IGNORED`, but return ignored files in
+	`ignored[]` in addition to untracked files in
+	`entries[]`. This flag is mutually exclusive with
+	`DIR_SHOW_IGNORED`.
 
 `DIR_KEEP_UNTRACKED_CONTENTS`:::
 
@@ -39,6 +43,21 @@ The notable options are:
 	untracked contents of untracked directories are also returned in
 	`entries[]`.
 
+`DIR_SHOW_IGNORED_TOO_MODE_MATCHING`:::
+
+	Only has meaning if `DIR_SHOW_IGNORED_TOO` is also set; if
+	this is set, returns ignored files and directories that match
+	an exclude pattern. If a directory matches an exclude pattern,
+	then the directory is returned and the contained paths are
+	not. A directory that does not match an exclude pattern will
+	not be returned even if all of its contents are ignored. In
+	this case, the contents are returned as individual entries.
++
+If this is set, files and directories that explicity match an ignore
+pattern are reported. Implicity ignored directories (directories that
+do not match an ignore pattern, but whose contents are all ignored)
+are not reported, instead all of the contents are reported.
+
 `DIR_COLLECT_IGNORED`:::
 
 	Special mode for git-add. Return ignored files in `ignored[]` and
-- 
2.13.6


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

* [PATCH v5 4/4] status: test ignored modes
  2017-10-30 17:21 ` [PATCH v5 0/4] status: add option to show ignored files differently jameson.miller81
                     ` (2 preceding siblings ...)
  2017-10-30 17:21   ` [PATCH v5 3/4] status: document options to show matching ignored files jameson.miller81
@ 2017-10-30 17:21   ` jameson.miller81
  2017-10-31  3:16   ` [PATCH v5 0/4] status: add option to show ignored files differently Junio C Hamano
  4 siblings, 0 replies; 49+ messages in thread
From: jameson.miller81 @ 2017-10-30 17:21 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, gitster, jamill, peff, sbeller

From: Jameson Miller <jamill@microsoft.com>

Add tests around status reporting ignord files that match an exclude
pattern for both --untracked-files=normal and --untracked-files=all.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
---
 t/t7521-ignored-mode.sh | 233 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 233 insertions(+)
 create mode 100755 t/t7521-ignored-mode.sh

diff --git a/t/t7521-ignored-mode.sh b/t/t7521-ignored-mode.sh
new file mode 100755
index 0000000000..91790943c3
--- /dev/null
+++ b/t/t7521-ignored-mode.sh
@@ -0,0 +1,233 @@
+#!/bin/sh
+
+test_description='git status ignored modes'
+
+. ./test-lib.sh
+
+test_expect_success 'setup initial commit and ignore file' '
+	cat >.gitignore <<-\EOF &&
+	*.ign
+	ignored_dir/
+	!*.unignore
+	EOF
+	git add . &&
+	git commit -m "Initial commit"
+'
+
+test_expect_success 'Verify behavior of status on directories with ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/ignored/ignored_1.ign
+	! dir/ignored/ignored_2.ign
+	! ignored/ignored_1.ign
+	! ignored/ignored_2.ign
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on directory with tracked & ignored files' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/tracked_ignored/ignored_1.ign
+	! dir/tracked_ignored/ignored_2.ign
+	! tracked_ignored/ignored_1.ign
+	! tracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p tracked_ignored dir/tracked_ignored &&
+	touch tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		tracked_ignored/ignored_1.ign tracked_ignored/ignored_2.ign \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 \
+		dir/tracked_ignored/ignored_1.ign dir/tracked_ignored/ignored_2.ign &&
+
+	git add tracked_ignored/tracked_1 tracked_ignored/tracked_2 \
+		dir/tracked_ignored/tracked_1 dir/tracked_ignored/tracked_2 &&
+	git commit -m "commit tracked files" &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on directory with untracked and ignored files' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? dir/untracked_ignored/untracked_1
+	? dir/untracked_ignored/untracked_2
+	? expect
+	? output
+	? untracked_ignored/untracked_1
+	? untracked_ignored/untracked_2
+	! dir/untracked_ignored/ignored_1.ign
+	! dir/untracked_ignored/ignored_2.ign
+	! untracked_ignored/ignored_1.ign
+	! untracked_ignored/ignored_2.ign
+	EOF
+
+	mkdir -p untracked_ignored dir/untracked_ignored &&
+	touch untracked_ignored/untracked_1 untracked_ignored/untracked_2 \
+		untracked_ignored/ignored_1.ign untracked_ignored/ignored_2.ign \
+		dir/untracked_ignored/untracked_1 dir/untracked_ignored/untracked_2 \
+		dir/untracked_ignored/ignored_1.ign dir/untracked_ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status matching ignored files on ignored directory' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored directory containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify matching ignored files with --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	? untracked_dir/
+	! ignored_dir/
+	! ignored_files/ignored_1.ign
+	! ignored_files/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir ignored_files untracked_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_files/ignored_1.ign ignored_files/ignored_2.ign \
+		untracked_dir/untracked &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify status behavior on ignored directory containing tracked file' '
+	test_when_finished "git clean -fdx && git reset HEAD~1 --hard" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! ignored_dir/ignored_1
+	! ignored_dir/ignored_1.ign
+	! ignored_dir/ignored_2
+	! ignored_dir/ignored_2.ign
+	EOF
+
+	mkdir ignored_dir &&
+	touch ignored_dir/ignored_1 ignored_dir/ignored_2 \
+		ignored_dir/ignored_1.ign ignored_dir/ignored_2.ign \
+		ignored_dir/tracked &&
+	git add -f ignored_dir/tracked &&
+	git commit -m "Force add file in ignored directory" &&
+	git status --porcelain=v2 --ignored=matching --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify behavior of status with --ignored=no' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=no --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify behavior of status with --ignored=traditional and --untracked-files=all' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/ignored/ignored_1.ign
+	! dir/ignored/ignored_2.ign
+	! ignored/ignored_1.ign
+	! ignored/ignored_2.ign
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=traditional --untracked-files=all >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'Verify behavior of status with --ignored=traditional and --untracked-files=normal' '
+	test_when_finished "git clean -fdx" &&
+	cat >expect <<-\EOF &&
+	? expect
+	? output
+	! dir/
+	! ignored/
+	EOF
+
+	mkdir -p ignored dir/ignored &&
+	touch ignored/ignored_1.ign ignored/ignored_2.ign \
+		dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign &&
+
+	git status --porcelain=v2 --ignored=traditional --untracked-files=normal >output &&
+	test_i18ncmp expect output
+'
+
+test_done
-- 
2.13.6


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

* Re: [PATCH v5 0/4] status: add option to show ignored files differently
  2017-10-30 17:21 ` [PATCH v5 0/4] status: add option to show ignored files differently jameson.miller81
                     ` (3 preceding siblings ...)
  2017-10-30 17:21   ` [PATCH v5 4/4] status: test ignored modes jameson.miller81
@ 2017-10-31  3:16   ` Junio C Hamano
  4 siblings, 0 replies; 49+ messages in thread
From: Junio C Hamano @ 2017-10-31  3:16 UTC (permalink / raw)
  To: jameson.miller81; +Cc: bmwill, git, jamill, peff, sbeller

jameson.miller81@gmail.com writes:

> Only difference from previous version is to update the commit author
> email to match corporate email address.

Will replace; thanks.

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

end of thread, other threads:[~2017-10-31  3:16 UTC | newest]

Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-05 20:54 [PATCH 0/6] Teach Status options around showing ignored files jameson.miller81
2017-10-05 20:54 ` [PATCH 1/6] Teach status " jameson.miller81
2017-10-05 20:54 ` [PATCH 2/6] Update documentation for new directory and status logic jameson.miller81
2017-10-05 21:51   ` Stefan Beller
2017-10-05 20:54 ` [PATCH 3/6] Add tests for git status `--ignored=matching` jameson.miller81
2017-10-05 21:59   ` Stefan Beller
2017-10-05 20:54 ` [PATCH 4/6] Expand support for ignored arguments on status jameson.miller81
2017-10-05 20:54 ` [PATCH 5/6] Add tests around status handling of ignored arguments jameson.miller81
2017-10-05 20:54 ` [PATCH 6/6] Handle unsupported combination status arguments jameson.miller81
2017-10-05 22:08   ` Stefan Beller
2017-10-05 21:16 ` [PATCH 0/6] Teach Status options around showing ignored files Jonathan Nieder
2017-10-05 21:22   ` Jameson Miller
2017-10-11 13:34 ` [PATCH v2 0/5] " Jameson Miller
2017-10-11 13:35   ` [PATCH v2 1/5] Teach status " Jameson Miller
2017-10-12  2:49     ` Junio C Hamano
2017-10-12 20:15       ` Jameson Miller
2017-10-11 13:35   ` [PATCH v2 2/5] Update documentation for new directory and status logic Jameson Miller
2017-10-12  2:55     ` Junio C Hamano
2017-10-12 20:54       ` Jameson Miller
2017-10-13  0:42         ` Junio C Hamano
2017-10-11 13:35   ` [PATCH v2 3/5] Add tests for git status `--ignored=matching` Jameson Miller
2017-10-11 13:35   ` [PATCH v2 4/5] Expand support for ignored arguments on status Jameson Miller
2017-10-12  3:58     ` Junio C Hamano
2017-10-11 13:35   ` [PATCH v2 5/5] Add tests around status handling of ignored arguments Jameson Miller
2017-10-12  4:06     ` Junio C Hamano
2017-10-12 20:16       ` Jameson Miller
2017-10-13  0:49         ` Junio C Hamano
2017-10-12  2:33   ` [PATCH v2 0/5] Teach Status options around showing ignored files Junio C Hamano
2017-10-12 20:20     ` Jameson Miller
2017-10-19 16:05 ` [PATCH v3 0/4] status: add option to show ignored files differently Jameson Miller
2017-10-19 16:05   ` [PATCH v3 1/4] " Jameson Miller
2017-10-23  3:34     ` Junio C Hamano
2017-10-19 16:05   ` [PATCH v3 2/4] status: report matching ignored and normal untracked Jameson Miller
2017-10-19 16:06   ` [PATCH v3 3/4] status: document options to show matching ignored files Jameson Miller
2017-10-19 16:06   ` [PATCH v3 4/4] status: test --ignored=matching Jameson Miller
2017-10-19 19:33     ` Stefan Beller
2017-10-22  2:10     ` Junio C Hamano
2017-10-23  4:46   ` [PATCH v3 0/4] status: add option to show ignored files differently Junio C Hamano
2017-10-23 17:05 ` [PATCH v4 " Jameson Miller
2017-10-23 17:05   ` [PATCH v4 1/4] " Jameson Miller
2017-10-23 17:05   ` [PATCH v4 2/4] status: report matching ignored and normal untracked Jameson Miller
2017-10-23 17:05   ` [PATCH v4 3/4] status: document options to show matching ignored files Jameson Miller
2017-10-23 17:05   ` [PATCH v4 4/4] status: test ignored modes Jameson Miller
2017-10-30 17:21 ` [PATCH v5 0/4] status: add option to show ignored files differently jameson.miller81
2017-10-30 17:21   ` [PATCH v5 1/4] " jameson.miller81
2017-10-30 17:21   ` [PATCH v5 2/4] status: report matching ignored and normal untracked jameson.miller81
2017-10-30 17:21   ` [PATCH v5 3/4] status: document options to show matching ignored files jameson.miller81
2017-10-30 17:21   ` [PATCH v5 4/4] status: test ignored modes jameson.miller81
2017-10-31  3:16   ` [PATCH v5 0/4] status: add option to show ignored files differently Junio C Hamano

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).