Re: [PATCH/RFC v2 0/2] add regex match flags to git describe

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 4, 2016, 17:46 UTC
Message-ID: <xmqqmvslp799.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <5684FE61.4010701@opera.com>

Mostyn Bramley-Moore <mostynb@opera.com> writes:

Show 15 quoted lines

> On 12/31/2015 01:23 AM, Junio C Hamano wrote:
> ...
>> Swapping the option key and value may not be a bad idea, but one
>> problem that the above does not solve, which I outlined in the
>> message you are responding to, is that "match-pattern-type" does not
>> give any hint that this is about affecting the match that is done to
>> "refs", e.g. you cannot tell in
>>
>>    $ git mgrep --match-pattern-type=perl-regexp -e foo --refs 'release_*'
>>
>> if the perl-regexp is to be used for matching branch names or for
>> matching the strings the command looks for in the trees of the
>> matching branches.
>
> There is a hint: --foo-pattern-type applies only to --foo.

Hmph.

Show 6 quoted lines

> In your mgrep example --match-pattern-type would apply to --match
> patterns only, and if we choose to implement it then
> --refs-pattern-type would apply to --refs patterns only.
> eg:
> $ git mgrep --match '^foo' --match-pattern-type=perl-regexp --refs
> release_*' --refs-pattern-type=glob

Most likely the hypothetical "mgrep" would not use "--match" but use "-e" to retain similarity to "grep", and "--e-pattern-type" would be confusing. But I agree that "--refs-pattern-type" uniformly used where we take pattern parameter on the command line to match with refs may make it clear that you are only affecting the matches against refs.

Show 16 quoted lines

>> Magic pattern annotation like we do for pathspecs Duy raised may not
>> be a bad idea, either, and would probably be easier to teach people.
>> Just like in Perl "(?i)$any_pattern" is a way to introduce the case
>> insensitive match with $any_pattern, we may be able to pick an
>> extensible magic syntax and decorate the pattern you would specify
>> for matching refnames to tell Git what kind of pattern it is, e.g.
>>
>>    $ git mgrep -P -e foo --refs '/?glob/release_*'
>>
>> I am not suggesting that we must use /?<pattern type name>/ prefix
>> as the "extensible magic syntax" here--I am just illustrating what
>> I mean by "extensible magic syntax".
>
> I hadn't seen the pathspec magic patterns before- interesting.  We
> could possibly share syntax with pathspecs, ie
> :(?pattern-options...)pattern

Even though we have DWIM between revisions and paths on the command line when the user omits "--" for disambiguation, I do not think we look at the shape of the string to DWIM/decide that it is a pattern, so as long as the magic syntax cannot be a valid pattern to match against refs right now (and your ":(?...)" qualifies as such, as a refname would not contain a component that begins with a colon), it would be possible to introduce it as the magic syntax for matching refs.

Or did you mean to use this syntax also for patterns that match strings in contents, e.g.

    $ git grep -e ':(?perl-regexp)...'

I am not bold enough to say that it would be a good idea, but I offhand do not think of a reason why we shouldn't go that route, either.

> Would this be confusing for commands that already have --perl-regexp
> etc?  What should happen if you specify both --perl-regexp and and a
> different type of pattern like :(glob)foo (error out, I suppose)?

If we were to go that route, ideally, I would say that

    $ git grep --perl-regexp -e 'A' -e ':(?basic-regexp)B' -e ':(?fixed-string)C'

should match with A as pcre, B as BRE and C as a fixed string.

I do not offhand remember if we built the underlying grep machinery in such a way that it is easy to extend it to allow such mixture, though.