[PATCH] revset: introduce the summary predicate

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sun Jan 8 07:59:36 EST 2017


(ha, I wrote my previous reply in a train and it got sent when I 
connected again (and received that one). I'm going to try to adress the 
new content in this email and sometime repeat some of my other reply 
content for clarity)

On 01/08/2017 04:23 AM, Matt Harbison wrote:
> On Sat, 07 Jan 2017 02:56:48 -0500, Yuya Nishihara <yuya at tcha.org> wrote:
>
>> On Fri, 6 Jan 2017 21:29:43 -0500, Matt Harbison wrote:
>>> > On Jan 6, 2017, at 11:19 AM, Pierre-Yves David
>>> <pierre-yves.david at ens-lyon.org> wrote:
>>> >> On 01/04/2017 07:04 PM, Matt Harbison wrote:
>>> >> # HG changeset patch
>>> >> # User Matt Harbison <matt_harbison at yahoo.com>
>>> >> # Date 1483550016 18000
>>> >> #      Wed Jan 04 12:13:36 2017 -0500
>>> >> # Node ID 76d95ab94b9e206363629059fb7824002e19a9e5
>>> >> # Parent  0064a1eb28e246ded9b726c696d048143d1b23f1
>>> >> revset: introduce the summary predicate
>>> >>
>>> >> This is similar to the 'desc()' predicate, however the search is
>>> limited to the
>>> >> first line, which I couldn't figure out how to do with the existing
>>> >> functionality.  Unlike 'desc()', this supports regular
>>> expressions.  The
>>> >> 'matching()' revset already treats the summary line distinctly
>>> from the full
>>> >> description, but that finds similar revisions instead of searching
>>> with a
>>> >> string.
>>> >
>>> > Looks like a great usecase to handle, I want that. However, are
>>> there reasons why:
>>> >
>>> > 1) We could not add full pattern support to desc()
>>> > 2) We could not make this an extra keyworkd parameters of desc()
>>> >
>>> > Multiplying the revset predicate hurts discoverability, having less
>>> but more powerful predicate seems useful.
>>>
>>> I share your concern about discoverability.
>>>
>>> I started out editing desc(), but it's explicitly documented as case
>>> insensitive.  It seems confusing if matching for literals is case
>>> insensitive, but regex is case sensitive for the same predicate. (I
>>> vaguely recall that you can make regex case insensitive, but I think
>>> that would also surprise the user.)  I meant to mention that maybe we
>>> could add pattern support to desc() in addition to this, but forgot.
>>> The other stuff I looked at that supports patterns, like tag(), is
>>> case sensitive for both literals and regex.  That makes sense, since
>>> those search for identifiers.
>>
>> Perhaps stringmatcher can have 3 types, icase literal, literal, and
>> re, and
>> the default of desc() is icase literal for backward compatibility. You
>> can
>> build a case-insensitive regexp object from a literal pattern.
>>
>> https://docs.python.org/2/library/re.html#re.I
>
> Yep, that's the API I was thinking of.
>
> I'm confused by the rest of your comments.  When I first skimmed your
> message, adding support for 'icasere:' using this API popped into my
> mind.  And that could support a case insensitive literal, because
> 'icasere:foo' should be equivalent to looking for the substring 'foo'
> (leaving aside efficiency, how discoverable that is, and that
> stringmatcher matches the whole string for literals).  But you seem to
> be suggesting adding 'icaseliteral:'.

I'm not 100% sure of what Yuya actually has in mind but here is my 
understanding of the situation and how we could move forward.

Currently:
----------

   desc(X) → X is customly matched as a case insensitive litteral,

   We have a "generic" pattern definition syntax used by various other 
reveset (implemented in "stringmatcher")

     foo(X)
       → X is matched as a case sensitive litteral
     foo('literal:X')
       → X is matched as a case sensitive literal (same as the above)
     food('re:X')
       → X is matched as a regular expression (case sensitive)

Proposal: (might be what yuya says)
---------

extend the string matcher to

   foo('literal:X')
     → X is matched as a case sensitive literal
   foo('icase-literal:X')
     → X is matched as a case insensitive literal
   food('re:X')
     → X is matched as a regular expression (case sensitive)

Then, desc move to use string matcher (default to "icase-literal").

We do not need a 'icase-re:' spec, because one can easily achieve it 
using 're:(?i)foo'

> […]
> I'm about to submit a patch to add the current 're:' support to 'desc'
> in the meantime, to hopefully move this along.

Great!

>  I'd also be curious if
> you have thoughts on how to conditionally limit this predicate to the
> first line, without limiting future functionality.

So having digged the regexp part a bit more, it seems like one could 
just use 're:^.*issue1337' to match "issue1337" on the first line ('.' 
does not match new-line by default).

If you think this is too obscure, what about:

     'desc(issue1337, firstline=True)'

See my other email for details.

Cheers,

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list