Manav Rathi <mnvrth@xxxxxxxxx> 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
