All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Mostyn Bramley-Moore <mostynb@opera.com>
Cc: git@vger.kernel.org, "brian m . carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH/RFC v2 0/2] add regex match flags to git describe
Date: Tue, 29 Dec 2015 10:27:09 -0800	[thread overview]
Message-ID: <xmqqk2nxi002.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <5681D02C.1040609@opera.com> (Mostyn Bramley-Moore's message of "Tue, 29 Dec 2015 01:13:32 +0100")

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

>> I do not think it is wrong per-se to add an option to use regular
>> expressions instead of globs, but if we are to do so, the endgame we
>> aim for MUST be that we do so consistently to all the other commands
>> that iterate over refs and limit their output to the ones that match
>> given pattern (or a set of patterns), not just 'describe'.
>
> There is one important distinction between 'git describe' and the
> other commands that iterate through refs- it applies an internal
> search strategy and outputs at most one match.  This makes it
> difficult to search for the closest matching tag...

If that was what you were trying to solve, then it sounds like a
typical XY problem.  You do not need custom matching flags; you need
a "give me N (or all) names based on possible tags" option.

And I do not think it is a bad thing to add.  I already
said that an option to match with a regular expression is not a bad
thing to add, either ;-)

> Besides 'git grep', the only regex type flag that is given a short
> option seems to be -E for 'git log' and 'git rev-list'.  I have no
> objection to dropping the short options, or leaving only -E.

They also take -F, but "log" and friends do not pattern match the
refnames, so I do not think you have to worry about them at the
moment.

It is more important to envision what we would do in the future when
a command that takes a pattern (or a set of patterns) to match the
refnames with _and_ another pattern (or a set of patterns) to match
something else, and take that into account when designing this
"allowing matching logic for refnames to be customized from glob to
something else" feature, so that we do not paint outselves into a
corner we cannot later get out of.  Imagine a hypothetical command
'git mgrep' that can look for a pattern in tips of multiple branches
that can be used this way:

    $ git mgrep -e 'froo*tz' --refs 'refs/heads/release/*'

which may look for strings that match "froo*tz" in the trees of
all branches whose name match the pattern 'release/*'.  In this
example, the pattern to match strings is a BRE (same default as 
"git grep"), and the pattern to match refnames is a glob.

Consistency & similarity with "git grep" would most likely lead us
to add -E/-F/-G/-P options to this command and to make it affect how
the pattern to match strings works.  For example:

    $ git mgrep -E -e 'fro+tz' --match-refs 'refs/heads/release/*'

may look for the same strings that would match the first example,
but the pattern is expressed in ERE.  "-P", "-G", and "-F" options
would also work the same way.

Now, the question is what this "-E" (or -P/-G/-F) should do with the
matching the command does with the refnames.  The easiest (and
laziest) way out from the implementors' point of view might be to
declare that they affect both at the same time.  But is that useful
in practice?  It probably isn't, as it forces the users to write

    $ git mgrep -E -e 'fro+tz' --match-refs 'refs/heads/release/.*'

because the ref matching suddenly starts to use ERE (not glob),
which most likely is not something users would expect.  So we may
need a separate set of options to affect the way how refs are
matched.

We cannot just say "but we do not have that 'mgrep' command yet, so
we can do whatever we want to do with 'describe' today".  When the
need eventually arises that requires us to be able to specify how
strings are matched and how refnames are matched independently, we
would end up with an inconsistent UI where 'describe' takes '-P' (or
'--perl-regexp') to affect the way how refnames are matched, while
commands like 'mgrep' would need to use '--refmatch-perl-regexp' (or
any other name that can be distinguished from '--perl-regexp') to do
the same thing because they do not want '--perl-regexp' to affect
the matching of refnames.

And at that point in the future, it is too late to fix 'describe',
as people are so used to use '--perl-regexp' to match with refs.  We
will forever regret that we did not give the option a name that can
be used independently from the existing '--perl-regexp' that is
about matching for strings, not refnames.

That is exactly the kind of thing that would paint us in a corner
that we cannot get out of, which we need to avoid, hence we need to
think ahead now.
 

  reply	other threads:[~2015-12-29 18:27 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 [this message]
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
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=xmqqk2nxi002.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mostynb@opera.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.