[PATCH] docs: improve discoverability of exclude pathspec

[PATCH] docs: improve discoverability of exclude pathspec

[Manav Rathi]

The ability to exclude paths in pathspecs is not mentioned in the man
pages of git grep and other commands where it might be useful.

Add a pointer to the pathspec syntax and a quick example in the git
grep man page to help the user to discover this ability.

Add similar pointers from the git-add and git-status man pages.

Additionally,

- Add a test for the behaviour when multiple exclusions are present.
- Add a test for the ^ alias.
- Perform general touch ups of surrounding lines.

Signed-off-by: Manav Rathi <mnvrth@gmail.com>
---
 Documentation/git-add.txt          | 27 ++++++++++++++++-----------
 Documentation/git-grep.txt         |  6 ++++++
 Documentation/git-status.txt       |  2 ++
 Documentation/glossary-content.txt |  2 +-
 t/t6132-pathspec-exclude.sh        | 13 ++++++++++++-
 5 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index f4169fb..6f76f39 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -50,17 +50,22 @@ commit.
 OPTIONS
 -------
 <pathspec>...::
- Files to add content from.  Fileglobs (e.g. `*.c`) can
- be given to add all matching files.  Also a
- leading directory name (e.g. `dir` to add `dir/file1`
- and `dir/file2`) can be given to update the index to
- match the current state of the directory as a whole (e.g.
- specifying `dir` will record not just a file `dir/file1`
- modified in the working tree, a file `dir/file2` added to
- the working tree, but also a file `dir/file3` removed from
- the working tree.  Note that older versions of Git used
- to ignore removed files; use `--no-all` option if you want
- to add modified or new files but ignore removed ones.
+ Files to add content from.
++
+File globs (e.g. `*.c`) can be given to add all matching files.  A
+leading directory name (e.g. `dir` to add `dir/file1` and `dir/file2`)
+can be given to update the index to match the current state of the
+directory as a whole.
++
+Note that specifying `dir` will record not just a file `dir/file1`
+modified in the working tree, a file `dir/file2` added to the working
+tree, but also a file `dir/file3` removed from the working tree.
+Older versions of Git used to ignore removed files; use the `--no-all`
+option if you want to add new and modified files but ignore removed
+ones.
++
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].

 -n::
 --dry-run::
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 720c785..18b4947 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -289,6 +289,9 @@ providing this option will cause it to die.
 <pathspec>...::
  If given, limit the search to paths matching at least one pattern.
  Both leading paths match and glob(7) patterns are supported.
++
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].

 Examples
 --------
@@ -305,6 +308,9 @@ Examples
  Looks for a line that has `NODE` or `Unexpected` in
  files that have lines that match both.

+`git grep solution -- :^Documentation`::
+ Looks for `solution`, excluding files in `Documentation`.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index d47f198..9f3a78a 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -111,6 +111,8 @@ configuration variable documented in linkgit:git-config[1].
  without options are equivalent to 'always' and 'never'
  respectively.

+<pathspec>...::
+ See the 'pathspec' entry in linkgit:gitglossary[7].

 OUTPUT
 ------
diff --git a/Documentation/glossary-content.txt
b/Documentation/glossary-content.txt
index b71b943..6b8888d 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -407,7 +407,7 @@ these forms:

 exclude;;
  After a path matches any non-exclude pathspec, it will be run
- through all exclude pathspec (magic signature: `!` or its
+ through all exclude pathspecs (magic signature: `!` or its
  synonym `^`). If it matches, the path is ignored.  When there
  is no non-exclude pathspec, the exclusion is applied to the
  result set as if invoked without any pathspec.
diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh
index 9dd5cde..7ce91ef 100755
--- a/t/t6132-pathspec-exclude.sh
+++ b/t/t6132-pathspec-exclude.sh
@@ -25,7 +25,7 @@ EOF
  test_cmp expect actual
 '

-test_expect_success 'exclude only no longer errors out' '
+test_expect_success 'exclude only pathspec uses default implicit pathspec' '
  git log --oneline --format=%s -- . ":(exclude)sub" >expect &&
  git log --oneline --format=%s -- ":(exclude)sub" >actual &&
  test_cmp expect actual
@@ -183,4 +183,15 @@ EOF
  test_cmp expect actual
 '

+test_expect_success 'multiple exclusions' '
+ git ls-files -- :^*/file2 :^sub2 >actual &&
+ cat <<EOF >expect &&
+file
+sub/file
+sub/sub/file
+sub/sub/sub/file
+EOF
+ test_cmp expect actual
+'
+
 test_done
-- 
2.10.1

Re: [PATCH] docs: improve discoverability of exclude pathspec

[Junio C Hamano]

Manav Rathi <mnvrth@gmail.com> writes:

> The ability to exclude paths in pathspecs is not mentioned in the man
> pages of git grep and other commands where it might be useful.

My reading stutters around "exclude paths in pathspecs" in the
above.  Perhaps "exclude paths with a negative pathspec" instead?

> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index f4169fb..6f76f39 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -50,17 +50,22 @@ commit.
>  OPTIONS
>  -------
>  <pathspec>...::
> - Files to add content from.  Fileglobs (e.g. `*.c`) can
> - be given to add all matching files.  Also a
> - leading directory name (e.g. `dir` to add `dir/file1`
> - and `dir/file2`) can be given to update the index to
> - match the current state of the directory as a whole (e.g.
> - specifying `dir` will record not just a file `dir/file1`
> - modified in the working tree, a file `dir/file2` added to
> - the working tree, but also a file `dir/file3` removed from
> - the working tree.  Note that older versions of Git used
> - to ignore removed files; use `--no-all` option if you want
> - to add modified or new files but ignore removed ones.
> + Files to add content from.
> ++
> +File globs (e.g. `*.c`) can be given to add all matching files.  A
> +leading directory name (e.g. `dir` to add `dir/file1` and `dir/file2`)
> +can be given to update the index to match the current state of the
> +directory as a whole.
> ++
> +Note that specifying `dir` will record not just a file `dir/file1`
> +modified in the working tree, a file `dir/file2` added to the working
> +tree, but also a file `dir/file3` removed from the working tree.
> +Older versions of Git used to ignore removed files; use the `--no-all`
> +option if you want to add new and modified files but ignore removed
> +ones.
> ++
> +For more details about the <pathspec> syntax, see the 'pathspec' entry
> +in linkgit:gitglossary[7].

This does a lot more than what the log message claims to do, unlike
the changes to other documentation pages.  Splitting the existing
paragraph in "git add" into multiple pagagraphs and changing a few
words here and there wasn't part of the bargain.

It would be easier to judge the merit of the patch if you split it
into two steps, if you want all the changes in it.  One would do
only what the log message claimed it did, i.e. "refer to glossary,
give an example where appropriate and add test".  That part I think
everybody can agree that it is a good change.  The change to the
introduction part I am not so sure about.

> -test_expect_success 'exclude only no longer errors out' '
> +test_expect_success 'exclude only pathspec uses default implicit pathspec' '

This is a very good change.

Back when the test was written, it was fresh in collective memory
that giving only negative pathspec elements without any positive
pathspec element resulted in an error, and that the behaviour was
updated so that it would not error out (and it was obvious that
implicit positive pattern to be used was to match all), but when
reading this with fresh eyes, "no longer errors out" is much much
less important than "start from include all and then subtract paths
that match".

>   git log --oneline --format=%s -- . ":(exclude)sub" >expect &&
>   git log --oneline --format=%s -- ":(exclude)sub" >actual &&
>   test_cmp expect actual
> @@ -183,4 +183,15 @@ EOF
>   test_cmp expect actual
>  '
>
> +test_expect_success 'multiple exclusions' '
> + git ls-files -- :^*/file2 :^sub2 >actual &&

Please quote these patterns inside "pair of dqs".

> + cat <<EOF >expect &&
> +file
> +sub/file
> +sub/sub/file
> +sub/sub/sub/file
> +EOF

By using <<-\EOF, you can indent (with tab) the contents of the here
document, like so:

	cat >expect <<-EOF &&
	file
	...
        EOF

By the way, please check your e-mail settings.  Your MUA seems to
have lost all tabs, and this patch does not apply.

Thanks.

> + test_cmp expect actual
> +'
> +
>  test_done

[PATCHv2] docs: improve discoverability of exclude pathspec

[Manav Rathi]

On Mon, Sep 25, 2017 at 10:03:49AM +0900, Junio C Hamano wrote:
> Manav Rathi <mnvrth@gmail.com> writes:
>
> > The ability to exclude paths in pathspecs is not mentioned in the man
> > pages of git grep and other commands where it might be useful.
> 
> My reading stutters around "exclude paths in pathspecs" in the
> above.  Perhaps "exclude paths with a negative pathspec" instead?

Changed

> > diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> 
> This does a lot more than what the log message claims to do, unlike
> the changes to other documentation pages.  Splitting the existing
> paragraph in "git add" into multiple pagagraphs and changing a few
> words here and there wasn't part of the bargain.
> 
> It would be easier to judge the merit of the patch if you split it
> into two steps, if you want all the changes in it.  One would do
> only what the log message claimed it did, i.e. "refer to glossary,
> give an example where appropriate and add test".  That part I think
> everybody can agree that it is a good change.  The change to the
> introduction part I am not so sure about.
> 

I have undone the auxiliary edits to git-add.txt.

> > -test_expect_success 'exclude only no longer errors out' '
> > +test_expect_success 'exclude only pathspec uses default implicit pathspec' '
> 
> This is a very good change.
> 

Thank you.

> > +test_expect_success 'multiple exclusions' '
> > + git ls-files -- :^*/file2 :^sub2 >actual &&
> 
> Please quote these patterns inside "pair of dqs".
> 

Done

> > + cat <<EOF >expect &&
> > +file
> > +sub/file
> > +sub/sub/file
> > +sub/sub/sub/file
> > +EOF
> 
> By using <<-\EOF, you can indent (with tab) the contents of the here
> document, like so:
> 
> 	cat >expect <<-EOF &&
> 	file
> 	...
>         EOF
> 

Done

> By the way, please check your e-mail settings.  Your MUA seems to
> have lost all tabs, and this patch does not apply.

I am sorry about that. The instructions in SubmittingPatches clearly
mentioned that this will happen, but still I was not careful enough.

Hopefully this one should not be broken.

-- >8 --

Subject: [PATCH v2] docs: improve discoverability of exclude pathspec

The ability to exclude paths with a negative pathspec is not mentioned
in the man pages for git grep and other commands where it might be
useful.

Add an example and a pointer to the pathspec glossary entry in the man
page for git grep to help the user to discover this ability.

Add similar pointers from the git-add and git-status man pages.

Additionally,

- Add a test for the behaviour when multiple exclusions are present.
- Add a test for the ^ alias.
- Improve name of existing test.
- Improve grammar in glossary description of the exclude pathspec.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Manav Rathi <mnvrth@gmail.com>
---
 Documentation/git-add.txt          |  3 +++
 Documentation/git-grep.txt         |  6 ++++++
 Documentation/git-status.txt       |  2 ++
 Documentation/glossary-content.txt |  2 +-
 t/t6132-pathspec-exclude.sh        | 13 ++++++++++++-
 5 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index f4169fb..b700bea 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -61,6 +61,9 @@ OPTIONS
 	the working tree.  Note that older versions of Git used
 	to ignore removed files; use `--no-all` option if you want
 	to add modified or new files but ignore removed	ones.
++
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].
 
 -n::
 --dry-run::
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 720c785..18b4947 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -289,6 +289,9 @@ providing this option will cause it to die.
 <pathspec>...::
 	If given, limit the search to paths matching at least one pattern.
 	Both leading paths match and glob(7) patterns are supported.
++
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].
 
 Examples
 --------
@@ -305,6 +308,9 @@ Examples
 	Looks for a line that has `NODE` or `Unexpected` in
 	files that have lines that match both.
 
+`git grep solution -- :^Documentation`::
+	Looks for `solution`, excluding files in `Documentation`.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index d47f198..9f3a78a 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -111,6 +111,8 @@ configuration variable documented in linkgit:git-config[1].
 	without options are equivalent to 'always' and 'never'
 	respectively.
 
+<pathspec>...::
+	See the 'pathspec' entry in linkgit:gitglossary[7].
 
 OUTPUT
 ------
diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index b71b943..6b8888d 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -407,7 +407,7 @@ these forms:
 
 exclude;;
 	After a path matches any non-exclude pathspec, it will be run
-	through all exclude pathspec (magic signature: `!` or its
+	through all exclude pathspecs (magic signature: `!` or its
 	synonym `^`). If it matches, the path is ignored.  When there
 	is no non-exclude pathspec, the exclusion is applied to the
 	result set as if invoked without any pathspec.
diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh
index 9dd5cde..eb829fc 100755
--- a/t/t6132-pathspec-exclude.sh
+++ b/t/t6132-pathspec-exclude.sh
@@ -25,7 +25,7 @@ EOF
 	test_cmp expect actual
 '
 
-test_expect_success 'exclude only no longer errors out' '
+test_expect_success 'exclude only pathspec uses default implicit pathspec' '
 	git log --oneline --format=%s -- . ":(exclude)sub" >expect &&
 	git log --oneline --format=%s -- ":(exclude)sub" >actual &&
 	test_cmp expect actual
@@ -183,4 +183,15 @@ EOF
 	test_cmp expect actual
 '
 
+test_expect_success 'multiple exclusions' '
+	git ls-files -- ":^*/file2" ":^sub2" >actual &&
+	cat <<-\EOF >expect &&
+	file
+	sub/file
+	sub/sub/file
+	sub/sub/sub/file
+	EOF
+	test_cmp expect actual
+'
+
 test_done
-- 
2.10.1

Re: [PATCHv2] docs: improve discoverability of exclude pathspec

[Junio C Hamano]

Manav Rathi <mnvrth@gmail.com> writes:

> Hopefully this one should not be broken.

Looks good.

Thanks.