Patchwork [3,of,9] mdiff.unidiff: add support for the noprefix option

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 13, 2014, 8:22 a.m.
Message ID <d03e758f55d169b24d7f.1415866931@devbig136.prn2.facebook.com>
Download mbox | patch
Permalink /patch/6702/
State Accepted
Headers show

Comments

Siddharth Agarwal - Nov. 13, 2014, 8:22 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1415863754 28800
#      Wed Nov 12 23:29:14 2014 -0800
# Node ID d03e758f55d169b24d7fa1ec6720ba4056917871
# Parent  9d05403982150c69e5c3e48e4968711d5fd68752
mdiff.unidiff: add support for the noprefix option
Martin von Zweigbergk - Nov. 13, 2014, 5:26 p.m.
Looks good, but missing test case. It seems like it should be testable at
this point.

On Thu Nov 13 2014 at 12:22:33 AM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1415863754 28800
> #      Wed Nov 12 23:29:14 2014 -0800
> # Node ID d03e758f55d169b24d7fa1ec6720ba4056917871
> # Parent  9d05403982150c69e5c3e48e4968711d5fd68752
> mdiff.unidiff: add support for the noprefix option
>
> diff --git a/mercurial/mdiff.py b/mercurial/mdiff.py
> --- a/mercurial/mdiff.py
> +++ b/mercurial/mdiff.py
> @@ -156,6 +156,13 @@
>
>      if not a and not b:
>          return ""
> +
> +    if opts.noprefix:
> +        aprefix = bprefix = ''
> +    else:
> +        aprefix = 'a/'
> +        bprefix = 'b/'
> +
>      epoch = util.datestr((0, 0))
>
>      fn1 = util.pconvert(fn1)
> @@ -170,17 +177,17 @@
>          if a is None:
>              l1 = '--- /dev/null%s' % datetag(epoch)
>          else:
> -            l1 = "--- %s%s" % ("a/" + fn1, datetag(ad, fn1))
> -        l2 = "+++ %s%s" % ("b/" + fn2, datetag(bd, fn2))
> +            l1 = "--- %s%s%s" % (aprefix, fn1, datetag(ad, fn1))
> +        l2 = "+++ %s%s" % (bprefix + fn2, datetag(bd, fn2))
>          l3 = "@@ -0,0 +1,%d @@\n" % len(b)
>          l = [l1, l2, l3] + ["+" + e for e in b]
>      elif not b:
>          a = splitnewlines(a)
> -        l1 = "--- %s%s" % ("a/" + fn1, datetag(ad, fn1))
> +        l1 = "--- %s%s%s" % (aprefix, fn1, datetag(ad, fn1))
>          if b is None:
>              l2 = '+++ /dev/null%s' % datetag(epoch)
>          else:
> -            l2 = "+++ %s%s" % ("b/" + fn2, datetag(bd, fn2))
> +            l2 = "+++ %s%s%s" % (bprefix, fn2, datetag(bd, fn2))
>          l3 = "@@ -1,%d +0,0 @@\n" % len(a)
>          l = [l1, l2, l3] + ["-" + e for e in a]
>      else:
> @@ -190,8 +197,8 @@
>          if not l:
>              return ""
>
> -        l.insert(0, "--- a/%s%s" % (fn1, datetag(ad, fn1)))
> -        l.insert(1, "+++ b/%s%s" % (fn2, datetag(bd, fn2)))
> +        l.insert(0, "--- %s%s%s" % (aprefix, fn1, datetag(ad, fn1)))
> +        l.insert(1, "+++ %s%s%s" % (bprefix, fn2, datetag(bd, fn2)))
>
>      for ln in xrange(len(l)):
>          if l[ln][-1] != '\n':
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - Nov. 13, 2014, 5:36 p.m.
On 11/13/2014 09:26 AM, Martin von Zweigbergk wrote:
> Looks good, but missing test case. It seems like it should be testable 
> at this point.

Not really -- the UI is only hooked up in patches 8 and 9.

>
> On Thu Nov 13 2014 at 12:22:33 AM Siddharth Agarwal <sid0@fb.com 
> <mailto:sid0@fb.com>> wrote:
>
>     # HG changeset patch
>     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
>     # Date 1415863754 28800
>     #      Wed Nov 12 23:29:14 2014 -0800
>     # Node ID d03e758f55d169b24d7fa1ec6720ba4056917871
>     # Parent  9d05403982150c69e5c3e48e4968711d5fd68752
>     mdiff.unidiff: add support for the noprefix option
>
>     diff --git a/mercurial/mdiff.py b/mercurial/mdiff.py
>     --- a/mercurial/mdiff.py
>     +++ b/mercurial/mdiff.py
>     @@ -156,6 +156,13 @@
>
>          if not a and not b:
>              return ""
>     +
>     +    if opts.noprefix:
>     +        aprefix = bprefix = ''
>     +    else:
>     +        aprefix = 'a/'
>     +        bprefix = 'b/'
>     +
>          epoch = util.datestr((0, 0))
>
>          fn1 = util.pconvert(fn1)
>     @@ -170,17 +177,17 @@
>              if a is None:
>                  l1 = '--- /dev/null%s' % datetag(epoch)
>              else:
>     -            l1 = "--- %s%s" % ("a/" + fn1, datetag(ad, fn1))
>     -        l2 = "+++ %s%s" % ("b/" + fn2, datetag(bd, fn2))
>     +            l1 = "--- %s%s%s" % (aprefix, fn1, datetag(ad, fn1))
>     +        l2 = "+++ %s%s" % (bprefix + fn2, datetag(bd, fn2))
>              l3 = "@@ -0,0 +1,%d @@\n" % len(b)
>              l = [l1, l2, l3] + ["+" + e for e in b]
>          elif not b:
>              a = splitnewlines(a)
>     -        l1 = "--- %s%s" % ("a/" + fn1, datetag(ad, fn1))
>     +        l1 = "--- %s%s%s" % (aprefix, fn1, datetag(ad, fn1))
>              if b is None:
>                  l2 = '+++ /dev/null%s' % datetag(epoch)
>              else:
>     -            l2 = "+++ %s%s" % ("b/" + fn2, datetag(bd, fn2))
>     +            l2 = "+++ %s%s%s" % (bprefix, fn2, datetag(bd, fn2))
>              l3 = "@@ -1,%d +0,0 @@\n" % len(a)
>              l = [l1, l2, l3] + ["-" + e for e in a]
>          else:
>     @@ -190,8 +197,8 @@
>              if not l:
>                  return ""
>
>     -        l.insert(0, "--- a/%s%s" % (fn1, datetag(ad, fn1)))
>     -        l.insert(1, "+++ b/%s%s" % (fn2, datetag(bd, fn2)))
>     +        l.insert(0, "--- %s%s%s" % (aprefix, fn1, datetag(ad, fn1)))
>     +        l.insert(1, "+++ %s%s%s" % (bprefix, fn2, datetag(bd, fn2)))
>
>          for ln in xrange(len(l)):
>              if l[ln][-1] != '\n':
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel@selenic.com <mailto:Mercurial-devel@selenic.com>
>     http://selenic.com/mailman/listinfo/mercurial-devel
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Martin von Zweigbergk - Nov. 13, 2014, 9:06 p.m.
On Thu Nov 13 2014 at 9:36:50 AM Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 11/13/2014 09:26 AM, Martin von Zweigbergk wrote:
> > Looks good, but missing test case. It seems like it should be testable
> > at this point.
>
> Not really -- the UI is only hooked up in patches 8 and 9.
>

I would prefer to have a first test added earlier, then additional tests
added in (the current) patch 4 and patch 6. I think I'm suggesting moving
those two patches last. With the current order, I have to look at patch 8
while looking at patch 4 and 6 to make sure the tests match. Does that make
sense?

Patch

diff --git a/mercurial/mdiff.py b/mercurial/mdiff.py
--- a/mercurial/mdiff.py
+++ b/mercurial/mdiff.py
@@ -156,6 +156,13 @@ 
 
     if not a and not b:
         return ""
+
+    if opts.noprefix:
+        aprefix = bprefix = ''
+    else:
+        aprefix = 'a/'
+        bprefix = 'b/'
+
     epoch = util.datestr((0, 0))
 
     fn1 = util.pconvert(fn1)
@@ -170,17 +177,17 @@ 
         if a is None:
             l1 = '--- /dev/null%s' % datetag(epoch)
         else:
-            l1 = "--- %s%s" % ("a/" + fn1, datetag(ad, fn1))
-        l2 = "+++ %s%s" % ("b/" + fn2, datetag(bd, fn2))
+            l1 = "--- %s%s%s" % (aprefix, fn1, datetag(ad, fn1))
+        l2 = "+++ %s%s" % (bprefix + fn2, datetag(bd, fn2))
         l3 = "@@ -0,0 +1,%d @@\n" % len(b)
         l = [l1, l2, l3] + ["+" + e for e in b]
     elif not b:
         a = splitnewlines(a)
-        l1 = "--- %s%s" % ("a/" + fn1, datetag(ad, fn1))
+        l1 = "--- %s%s%s" % (aprefix, fn1, datetag(ad, fn1))
         if b is None:
             l2 = '+++ /dev/null%s' % datetag(epoch)
         else:
-            l2 = "+++ %s%s" % ("b/" + fn2, datetag(bd, fn2))
+            l2 = "+++ %s%s%s" % (bprefix, fn2, datetag(bd, fn2))
         l3 = "@@ -1,%d +0,0 @@\n" % len(a)
         l = [l1, l2, l3] + ["-" + e for e in a]
     else:
@@ -190,8 +197,8 @@ 
         if not l:
             return ""
 
-        l.insert(0, "--- a/%s%s" % (fn1, datetag(ad, fn1)))
-        l.insert(1, "+++ b/%s%s" % (fn2, datetag(bd, fn2)))
+        l.insert(0, "--- %s%s%s" % (aprefix, fn1, datetag(ad, fn1)))
+        l.insert(1, "+++ %s%s%s" % (bprefix, fn2, datetag(bd, fn2)))
 
     for ln in xrange(len(l)):
         if l[ln][-1] != '\n':