Patchwork [3,of,3,stable] extdiff: fix wrong comment about regexp ordering - regexps are just greedy

login
register
mail settings
Submitter Mads Kiilerich
Date Jan. 27, 2015, 2:15 a.m.
Message ID <02b0b226f33ba22df7e8.1422324952@ssl.google-analytics.com>
Download mbox | patch
Permalink /patch/7558/
State Deferred
Headers show

Comments

Mads Kiilerich - Jan. 27, 2015, 2:15 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1422324901 -3600
#      Tue Jan 27 03:15:01 2015 +0100
# Branch stable
# Node ID 02b0b226f33ba22df7e820d27fe93a79b69ad24a
# Parent  bf99a88b77b67ac4c88674ba5d1d19dd97092426
extdiff: fix wrong comment about regexp ordering - regexps are just greedy
Yuya Nishihara - Jan. 27, 2015, 2:41 p.m.
On Tue, 27 Jan 2015 03:15:52 +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1422324901 -3600
> #      Tue Jan 27 03:15:01 2015 +0100
> # Branch stable
> # Node ID 02b0b226f33ba22df7e820d27fe93a79b69ad24a
> # Parent  bf99a88b77b67ac4c88674ba5d1d19dd97092426
> extdiff: fix wrong comment about regexp ordering - regexps are just greedy
> 
> diff --git a/hgext/extdiff.py b/hgext/extdiff.py
> --- a/hgext/extdiff.py
> +++ b/hgext/extdiff.py
> @@ -218,9 +218,8 @@ def dodiff(ui, repo, cmdline, pats, opts
>                  return pre
>              return pre + util.shellquote(replace[key])
>  
> -        # Match parent2 first, so 'parent1?' will match both parent1 and parent
>          regex = (r'''(['"]?)([^ '"]*)?'''
> -                 r'\$(parent2|parent1?|child|plabel1|plabel2|clabel|root)\1')
> +                 r'\$(parent1?|parent2|child|plabel1|plabel2|clabel|root)\1')

IIRC, regexp is greedy, but the first matching pattern is used. So "$parent2"
is eaten by "\$(parent1?|".

I think "parent[12]?" can kill the confusing comment.

(original)

  % ./hg kdiff --config merge-tools.kdiff3.diffargs='$parent2' --debug
  running '/usr/bin/kdiff3 ' in /tmp/extdiff.NSwCFZ

(with this patch)

  % ./hg kdiff --config merge-tools.kdiff3.diffargs='$parent2' --debug
  running '/usr/bin/kdiff3 /tmp/extdiff.x_goam/mercurial-work.02b0b226f33b
  /hgext/extdiff.py2' in /tmp/extdiff.x_goam

Regards,
Matt Mackall - Jan. 27, 2015, 10:38 p.m.
On Tue, 2015-01-27 at 03:15 +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1422324901 -3600
> #      Tue Jan 27 03:15:01 2015 +0100
> # Branch stable
> # Node ID 02b0b226f33ba22df7e820d27fe93a79b69ad24a
> # Parent  bf99a88b77b67ac4c88674ba5d1d19dd97092426
> extdiff: fix wrong comment about regexp ordering - regexps are just greedy

Confused, is the regex buggy? If so, what is the bug? If not, why is it
changing?

> -        # Match parent2 first, so 'parent1?' will match both parent1 and parent
>          regex = (r'''(['"]?)([^ '"]*)?'''
> -                 r'\$(parent2|parent1?|child|plabel1|plabel2|clabel|root)\1')
> +                 r'\$(parent1?|parent2|child|plabel1|plabel2|clabel|root)\1')

Patch

diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -218,9 +218,8 @@  def dodiff(ui, repo, cmdline, pats, opts
                 return pre
             return pre + util.shellquote(replace[key])
 
-        # Match parent2 first, so 'parent1?' will match both parent1 and parent
         regex = (r'''(['"]?)([^ '"]*)?'''
-                 r'\$(parent2|parent1?|child|plabel1|plabel2|clabel|root)\1')
+                 r'\$(parent1?|parent2|child|plabel1|plabel2|clabel|root)\1')
         if not do3way and not re.search(regex, cmdline):
             cmdline += ' $parent1 $child'
         cmdline = re.sub(regex, quote, cmdline)