Re: [PATCH v3 3/5] name-rev: add support to exclude refs by pattern match

From: Jacob Keller <jacob.keller@gmail.com>
Date: Jan 18, 2017, 22:31 UTC
Message-ID: <CA+P7+xo3bS0EfXaVCZbd6NwbBA8vP5HiZNpAh_4v7S_p2jm+Zg@mail.gmail.com>
In-Reply-To: <xmqqy3y82fs8.fsf@gitster.mtv.corp.google.com>

On Wed, Jan 18, 2017 at 1:56 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

> Jacob Keller <jacob.keller@gmail.com> writes:
>
>> Yes this makes sense. I'm still looking at whether the alternative
>> implementation suggested based on the git-log style would make more
>> sense or not, but if we keep this as is, the added text you gave is
>> important.
>
> I actually think it is a red-herring that "git log" honors "orders";
> it does, but that is not a result of carefully considering the
> desired behaviour.  It instead is a historical wart that came from
> the fact that "--branches" and friends uses for_each_glob_ref_in()
> that takes the top-level hierarchy paths like "refs/heads/" and the
> implementation of "--exclude" piggybacked into the function in a
> lazy way.
>
> If exclusion were done independently (e.g. in a way similar to what
> you did in this series using subpath match), we wouldn't have had
> the "the user must give exclude patterns first that would affect the
> next inclusion pattern, at which point the exclude patterns are
> cleared and the user needs to start over", which is an end-user
> experience that is clunky.
>

However, it is useful that exclude patterns only apply to specific match parameters? That is the advantage of the other implementation.

I think I agree that it's not really worth the complexity, as it requires a much more complex explanation of how the parameters interact, and in general doesn't provide that much more expressiveness, since at least for "git describe" by definition it either finds the tag as a match or not. Sure you could say "include all tags matching x but only if they don't match y" and include all tags matching z even if they match y" using that mechanism, but I think that makes the entire thing needlessly more complicated than "we use a tag if it matches any match and doesn't match any exclude".

Thanks, Jake