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