On Wed, Jan 18, 2017 at 12:04 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Jacob Keller <jacob.e.keller@xxxxxxxxx> writes:
>
>> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>>
>> Teach git name-rev to take a string list of patterns from --refs instead
>> of only a single pattern. The list of patterns will be matched
>> inclusively, such that a ref only needs to match one pattern to be
>> included. If a ref will only be excluded if it does not match any of the
>> patterns.
>
> I think "If a" in the last sentence should be "A".
You are correct, that is a typo.
>
>> --refs=<pattern>::
>> Only use refs whose names match a given shell pattern. The pattern
>> - can be one of branch name, tag name or fully qualified ref name.
>> + can be one of branch name, tag name or fully qualified ref name. If
>> + given multiple times, use refs whose names match any of the given shell
>> + patterns. Use `--no-refs` to clear any previous ref patterns given.
>
> Unlike 1/5, this is facing the end-users, and the last sentence is
> very appropriate.
Yes.
>
>> + if (data->ref_filters.nr) {
>> + struct string_list_item *item;
>> + int matched = 0;
>> +
>> + /* See if any of the patterns match. */
>> + for_each_string_list_item(item, &data->ref_filters) {
>> + /*
>> + * We want to check every pattern even if we already
>> + * found a match, just in case one of the later
>> + * patterns could abbreviate the output.
>> + */
>> + switch (subpath_matches(path, item->string)) {
>> + case -1: /* did not match */
>> + break;
>> + case 0: /* matched fully */
>> + matched = 1;
>> + break;
>> + default: /* matched subpath */
>> + matched = 1;
>> + can_abbreviate_output = 1;
>> + break;
>> + }
>> }
>
> I agree that we cannot short-cut on the first match to make sure
> that the outcome is stable, but I wondered what should be shown when
> we do have multiple matches. Say I gave
>
> --refs="v*" --refs="refs/tags/v1.*"
>
> and refs/tags/v1.0 matched. The above code would say we can
> abbreviate.
>
> What is the reason behind this design decision? Is it because it is
> clear that the user shows her willingness to accept more compact
> form by having --refs="v*" that would allow shortening? If that is
> the case, I think I agree with the reasoning. But we probably want
> to write it down somewhere, because another reasoning, which may
> also be valid, would call for an opposite behaviour (i.e. the more
> specific --refs="refs/tags/v1.*" also matched, so let's show that
> fact by not shortening).
>
I'm not sure which reasoning makes most sense. Any other opinions?
Thanks,
Jake