[PATCH] revset: introduce the summary predicate

[Matt Harbison]

On Sun, 08 Jan 2017 07:59:36 -0500, Pierre-Yves David  
<pierre-yves.david at ens-lyon.org> wrote:

> (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
>>>> >>

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

See the comment in the new patch I sent about 'user()' already lowercasing  
'literal:' and 're:'.  I'd consider it a bug, but it's been in since mid  
2012.  Attempting to channel Matt, I'm guessing we are stuck with that  
since it is so old, but wanted to see what others think.

>    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'

Ah! I missed the part in the docs where flags could be set in the string  
with (?<flag>). I thought you needed to compile with re.FLAG.  When he  
said string literal, my mind went right to the 'literal:' prefix.  Agreed,  
no need for 'icase-re:'.

>> […]
>> 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).

Thanks for looking at that.  It's way less horrible than I thought it  
would be.  I'm curious what Sean thinks, since he mentioned {firstline}  
being put in as a substitute for a complex regex.  I'd be fine with  
skipping the firstline=True param if this case is mentioned in the help  
for desc().

(I've also got a documentation update that eliminates all of these copies  
of how 're:' and 'literal:' works, once the dust settles on this.)

> If you think this is too obscure, what about:
>
>      'desc(issue1337, firstline=True)'
>
> See my other email for details.
>
> Cheers,