Re: [PATCH/RFC v2 0/2] add regex match flags to git describe
- From
- Mostyn Bramley-Moore <mostynb@opera.com>
- Date
- Jan 6, 2016, 01:08 UTC
- Message-ID
- <568C690C.7050007@opera.com>
- In-Reply-To
- <xmqqmvslp799.fsf@gitster.mtv.corp.google.com>
On 01/04/2016 06:46 PM, Junio C Hamano wrote:
Show 33 quoted lines
> Mostyn Bramley-Moore <mostynb@opera.com> writes: > >> 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. > >> 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.
I don't think -e foo --e-pattern-type=bar would be confusing.
Show 30 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 think it would be nice to support this syntax in every command that does pattern matching.
Corner case: what if we want to search for ":(?perl-regexp)", eg in git's own source? I suppose this would do: git grep -e ":(?fixed-strings):(?perl-regexp)"
Show 13 quoted lines
> 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.
That makes sense.
> 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.
I believe this would require some moderate refactoring, but I have only looked briefly so far. If we can settle on a preliminary design (--foo-pattern-type vs magic pattern option strings), I can try implementing a proof-of-concept.
-Mostyn.
-- Mostyn Bramley-Moore TV and Connected Devices Opera Software ASA mostynb@opera.com