Re: [PATCH 3/5] refspec: output a refspec item

[Derrick Stolee <stolee@gmail.com>]

On 4/7/2021 4:46 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Apr 05 2021, Derrick Stolee via GitGitGadget wrote:
>> +	return buf.buf;
> 
> There's a downthread discussion about the strbuf usage here so that's
> covered.

And it's fixed in v2.

> But I'm still confused about the need for this function and the
> following two patches. If we apply this on top of your series:
>     
>     diff --git a/t/helper/test-refspec.c b/t/helper/test-refspec.c
>     index 08cf441a0a0..9e099e43ebf 100644
>     --- a/t/helper/test-refspec.c
>     +++ b/t/helper/test-refspec.c
>     @@ -31,7 +31,7 @@ int cmd__refspec(int argc, const char **argv)
>                             continue;
>                     }
>     
>     -               printf("%s\n", refspec_item_format(&rsi));
>     +               puts(line.buf);
>                     refspec_item_clear(&rsi);
>             }
> 
> The only failing test is:
>     
>     + diff -u expect output
>     --- expect      2021-04-07 08:12:05.577598038 +0000
>     +++ output      2021-04-07 08:12:05.577598038 +0000
>     @@ -11,5 +11,5 @@
>      refs/heads*/for-linus:refs/remotes/mine/*
>      2e36527f23b7f6ae15e6f21ac3b08bf3fed6ee48:refs/heads/fixed
>      HEAD
>     -HEAD
>     +@
>      :

It should be obvious that taking refspecs as input, parsing them,
then reformatting them for output should be almost equivalent to
printing the input line.

The point is to exercise the logic that actually formats the
refspec for output. The test-tool clearly does this.

The logic for converting a 'struct refspec_item' to a string is
non-trivial and worth testing. I don't understand why you are
concerned that the black-box of the test-tool could be done
more easily to "trick" the test script.

> So the purpose of this new API is that we don't want to make the
> assumption that strrchr(buf, ':') is a safe way to find the delimiter in
> the refspec, or is there some case where we grok "HEAD" but not "@"
> that's buggy, but not tested for in this series?

The purpose is to allow us to modify a 'struct refspec_item' andproduce a refspec string instead of munging a refspec string
directly.

Thanks,
-Stolee