git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mostyn Bramley-Moore <mostynb@opera.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Duy Nguyen <pclouds@gmail.com>,
	"brian m . carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH/RFC v2 0/2] add regex match flags to git describe
Date: Wed, 6 Jan 2016 02:08:28 +0100	[thread overview]
Message-ID: <568C690C.7050007@opera.com> (raw)
In-Reply-To: <xmqqmvslp799.fsf@gitster.mtv.corp.google.com>

On 01/04/2016 06:46 PM, Junio C Hamano wrote:
> 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.

>>> 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)"

> 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

  reply	other threads:[~2016-01-06  1:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-28 10:30 [PATCH/RFC v2 0/2] add regex match flags to git describe Mostyn Bramley-Moore
2015-12-28 10:30 ` [PATCH/RFC v2 1/2] describe: add option to use perl-compatible regexes with --match Mostyn Bramley-Moore
2015-12-28 10:30 ` [PATCH/RFC v2 2/2] describe: add basic and extended posix regex matching for completeness Mostyn Bramley-Moore
2015-12-28 20:30 ` [PATCH/RFC v2 0/2] add regex match flags to git describe Junio C Hamano
2015-12-29  0:13   ` Mostyn Bramley-Moore
2015-12-29 18:27     ` Junio C Hamano
2015-12-30  9:52       ` Duy Nguyen
2015-12-31  0:08         ` Mostyn Bramley-Moore
2015-12-31  0:00       ` Mostyn Bramley-Moore
2015-12-31  0:23         ` Junio C Hamano
2015-12-31 10:07           ` Mostyn Bramley-Moore
2016-01-04 17:46             ` Junio C Hamano
2016-01-06  1:08               ` Mostyn Bramley-Moore [this message]
2016-01-06 12:23                 ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=568C690C.7050007@opera.com \
    --to=mostynb@opera.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).