Re: [PATCHv9 4/4] pathspec: allow querying for attributes

[Stefan Beller <sbeller@google.com>]

On Fri, May 20, 2016 at 11:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> +attr;;
>>> +After `attr:` comes a space separated list of "attribute
>>> +...
>>> ++
>>
>> The text looks OK, but does it format well?
>
> I didn't check this, but the remainder would look like this
> squashable patch.

I checked and it looks wrong. the "exclude" section is indented below
the new attr section

fix is:
--8<--
diff --git a/Documentation/glossary-content.txt
b/Documentation/glossary-content.txt
index e06520b..181f52e 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -389,7 +389,7 @@ After `attr:` comes a space separated list of "attribute
 requirements", all of which must be met in order for the
 path to be considered a match; this is in addition to the
 usual non-magic pathspec pattern matching.
-
++
 Each of the attribute requirements for the path takes one of
 these forms:
--8<--

I can resend with your proposed fixes as well.

Thanks,
Stefan



>
> You seem to i18ngrep for "fatal" but we are using test_must_fail for
> the exit status, so I am not sure if that adds much value, so the
> additional tests here do nto use that pattern.
>
> diff --git a/pathspec.c b/pathspec.c
> index 693a5e7..0a02255 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -115,19 +115,19 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va
>                 const char *attr = si->string;
>                 struct attr_match *am = &item->attr_match[j];
>
> -               attr_len = strcspn(attr, "=");
>                 switch (*attr) {
>                 case '!':
>                         am->match_mode = MATCH_UNSPECIFIED;
>                         attr++;
> -                       attr_len--;
> +                       attr_len = strlen(attr);
>                         break;
>                 case '-':
>                         am->match_mode = MATCH_UNSET;
>                         attr++;
> -                       attr_len--;
> +                       attr_len = strlen(attr);
>                         break;
>                 default:
> +                       attr_len = strcspn(attr, "=");
>                         if (attr[attr_len] != '=')
>                                 am->match_mode = MATCH_SET;
>                         else {
> diff --git a/t/t6134-pathspec-with-labels.sh b/t/t6134-pathspec-with-labels.sh
> index 5da1a63..060047a 100755
> --- a/t/t6134-pathspec-with-labels.sh
> +++ b/t/t6134-pathspec-with-labels.sh
> @@ -160,4 +160,9 @@ test_expect_success 'abort on giving invalid label on the command line' '
>         test_i18ngrep "fatal" actual
>  '
>
> +test_expect_success 'abort on asking for wrong magic' '
> +       test_must_fail git ls-files . ":(attr:-label=foo)" &&
> +       test_must_fail git ls-files . ":(attr:!label=foo)"
> +'
> +
>  test_done