Thread (2 messages) flat view 2 messages, 2 authors, 2017-01-18

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

From: Jacob Keller <hidden>
Date: 2017-01-18 22:41:13

On Wed, Jan 18, 2017 at 1:56 PM, Junio C Hamano [off-list ref] wrote:
Jacob Keller [off-list ref] writes:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help