Joanna Wang <jojwang@xxxxxxxxxx> writes:
> Added 'add:' and specified it is 'patchpsec' magic, but lmk if i'm still missing something.
If you run
$ git shortlog --no-merges v2.42.0..
or something on your branch to see how well its subject blends with
others, you'd start wanting to (1) downcase "Enable", and (2) omit
the full-stop after the title, to make it look similar to others.
> diff --git a/builtin/stash.c b/builtin/stash.c
> index 1ad496985a..9c77d3e4e4 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1760,7 +1760,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,
> }
> }
>
> - parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,
> + parse_pathspec(&ps, PATHSPEC_ATTR, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,
> prefix, argv);
This becomes necessary because "stash" later goes through other
places this patch touches (e.g., dir.c::exclude_matches_pathspec()?)
that happened to be serving as a guard for another code "stash" has
that is not prepared to handle attribute magic? Do you know what
exactly is not ready, so that perhaps others can help figuring out
how to make it ready for the attr magic?
> diff --git a/dir.c b/dir.c
> index 8486e4d56f..9bf9b53ca5 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2174,12 +2174,13 @@ static int exclude_matches_pathspec(const char *path, int pathlen,
> return 0;
>
> GUARD_PATHSPEC(pathspec,
> - PATHSPEC_FROMTOP |
> - PATHSPEC_MAXDEPTH |
> - PATHSPEC_LITERAL |
> - PATHSPEC_GLOB |
> - PATHSPEC_ICASE |
> - PATHSPEC_EXCLUDE);
> + PATHSPEC_FROMTOP |
> + PATHSPEC_MAXDEPTH |
> + PATHSPEC_LITERAL |
> + PATHSPEC_GLOB |
> + PATHSPEC_ICASE |
> + PATHSPEC_EXCLUDE |
> + PATHSPEC_ATTR);
Why this reindent?
> @@ -239,16 +254,100 @@ test_expect_success 'fail on multiple attr specifie
> test_i18ngrep "Only one" actual
> '
>
> -test_expect_success 'fail if attr magic is used places not implemented' '
> +test_expect_success 'fail if attr magic is used in places not implemented' '
> # The main purpose of this test is to check that we actually fail
> # when you attempt to use attr magic in commands that do not implement
> - # attr magic. This test does not advocate git-add to stay that way,
> - # though, but git-add is convenient as it has its own internal pathspec
> - # parsing.
> - test_must_fail git add ":(attr:labelB)" 2>actual &&
> + # attr magic. This test does not advocate stash push to stay that way.
> + # When you teach the command to grok the pathspec, you need to find
> + # another commnad to replace it for the test.
There is a typo here. Please do not expect your reviewers to always
offer a perfect solution to your problem and blindly copy what they
fed you. Instead, just like other project participants try to find
bugs and improvement opportunities in your patch, please lend them
sanity-checking eyeballs in return ;-)
