Patchwork [6,of,6,for-clowncopter] trydiff: join elements in 'header' list by '\n'

login
register
mail settings
Submitter Martin von Zweigbergk
Date Feb. 3, 2015, 4:58 p.m.
Message ID <0a9b46f2acc8dd79fb2a.1422982712@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/7629/
State Accepted
Commit bbb011f4eb32769c26b4f76e11b941aa845ec20e
Headers show

Comments

Martin von Zweigbergk - Feb. 3, 2015, 4:58 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1421451243 28800
#      Fri Jan 16 15:34:03 2015 -0800
# Node ID 0a9b46f2acc8dd79fb2afb1d7b202820cd8ae9a2
# Parent  1522a50d388c8284f16d22b9e01a087966c35f5f
trydiff: join elements in 'header' list by '\n'

It seems natural that each element in the list corresponds to one line
of output. That is currently true, but only because each element in
the list has a trailing newline. Let's drop those newlines and instead
add them when we print the headers.
Augie Fackler - Feb. 3, 2015, 8:13 p.m.
On Tue, Feb 03, 2015 at 08:58:32AM -0800, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1421451243 28800
> #      Fri Jan 16 15:34:03 2015 -0800
> # Node ID 0a9b46f2acc8dd79fb2afb1d7b202820cd8ae9a2
> # Parent  1522a50d388c8284f16d22b9e01a087966c35f5f
> trydiff: join elements in 'header' list by '\n'

This series LGTM, I think it should go to the clowncopter.

>
> It seems natural that each element in the list corresponds to one line
> of output. That is currently true, but only because each element in
> the list has a trailing newline. Let's drop those newlines and instead
> add them when we print the headers.
>
> diff -r 1522a50d388c -r 0a9b46f2acc8 mercurial/patch.py
> --- a/mercurial/patch.py	Fri Jan 16 15:27:04 2015 -0800
> +++ b/mercurial/patch.py	Fri Jan 16 15:34:03 2015 -0800
> @@ -1755,7 +1755,7 @@
>
>      def diffline(f, revs):
>          revinfo = ' '.join(["-r %s" % rev for rev in revs])
> -        return 'diff %s %s\n' % (revinfo, f)
> +        return 'diff %s %s' % (revinfo, f)
>
>      date1 = util.datestr(ctx1.date())
>      date2 = util.datestr(ctx2.date())
> @@ -1836,34 +1836,34 @@
>          path2 = posixpath.join(prefix, f2)
>          header = []
>          if opts.git:
> -            header.append('diff --git %s%s %s%s\n' %
> +            header.append('diff --git %s%s %s%s' %
>                            (aprefix, path1, bprefix, path2))
>              if content1 is None: # added
> -                header.append('new file mode %s\n' % gitmode[flag2])
> +                header.append('new file mode %s' % gitmode[flag2])
>              elif content2 is None: # removed
> -                header.append('deleted file mode %s\n' % gitmode[flag1])
> +                header.append('deleted file mode %s' % gitmode[flag1])
>              else:  # modified/copied/renamed
>                  mode1, mode2 = gitmode[flag1], gitmode[flag2]
>                  if mode1 != mode2:
> -                    header.append('old mode %s\n' % mode1)
> -                    header.append('new mode %s\n' % mode2)
> +                    header.append('old mode %s' % mode1)
> +                    header.append('new mode %s' % mode2)
>                  if op is not None:
> -                    header.append('%s from %s\n' % (op, path1))
> -                    header.append('%s to %s\n' % (op, path2))
> +                    header.append('%s from %s' % (op, path1))
> +                    header.append('%s to %s' % (op, path2))
>          elif revs and not repo.ui.quiet:
>              header.append(diffline(path1, revs))
>
>          if binarydiff and not opts.nobinary:
>              text = mdiff.b85diff(content1, content2)
>              if text and opts.git:
> -                header.append('index %s..%s\n' %
> +                header.append('index %s..%s' %
>                                (gitindex(content1), gitindex(content2)))
>          else:
>              text = mdiff.unidiff(content1, date1,
>                                   content2, date2,
>                                   path1, path2, opts=opts)
>          if header and (text or len(header) > 1):
> -            yield ''.join(header)
> +            yield '\n'.join(header) + '\n'
>          if text:
>              yield text
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - Feb. 4, 2015, 1:01 a.m.
On Tue, 2015-02-03 at 08:58 -0800, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1421451243 28800
> #      Fri Jan 16 15:34:03 2015 -0800
> # Node ID 0a9b46f2acc8dd79fb2afb1d7b202820cd8ae9a2
> # Parent  1522a50d388c8284f16d22b9e01a087966c35f5f
> trydiff: join elements in 'header' list by '\n'

These are queued for default, thanks.

(I'm not sure what it means that they're flagged for clowncopter,
they're still taking up space in my inbox and I still have to review
them eventually.)
Augie Fackler - Feb. 4, 2015, 9:05 p.m.
On Tue, Feb 3, 2015 at 8:01 PM, Matt Mackall <mpm@selenic.com> wrote:
> (I'm not sure what it means that they're flagged for clowncopter,
> they're still taking up space in my inbox and I still have to review
> them eventually.)


I believe that was a signal that they only (at the time) applied
against clowncopter, and that they shouldn't be reviewed unless you
know you've pulled clowncopter.

Patch

diff -r 1522a50d388c -r 0a9b46f2acc8 mercurial/patch.py
--- a/mercurial/patch.py	Fri Jan 16 15:27:04 2015 -0800
+++ b/mercurial/patch.py	Fri Jan 16 15:34:03 2015 -0800
@@ -1755,7 +1755,7 @@ 
 
     def diffline(f, revs):
         revinfo = ' '.join(["-r %s" % rev for rev in revs])
-        return 'diff %s %s\n' % (revinfo, f)
+        return 'diff %s %s' % (revinfo, f)
 
     date1 = util.datestr(ctx1.date())
     date2 = util.datestr(ctx2.date())
@@ -1836,34 +1836,34 @@ 
         path2 = posixpath.join(prefix, f2)
         header = []
         if opts.git:
-            header.append('diff --git %s%s %s%s\n' %
+            header.append('diff --git %s%s %s%s' %
                           (aprefix, path1, bprefix, path2))
             if content1 is None: # added
-                header.append('new file mode %s\n' % gitmode[flag2])
+                header.append('new file mode %s' % gitmode[flag2])
             elif content2 is None: # removed
-                header.append('deleted file mode %s\n' % gitmode[flag1])
+                header.append('deleted file mode %s' % gitmode[flag1])
             else:  # modified/copied/renamed
                 mode1, mode2 = gitmode[flag1], gitmode[flag2]
                 if mode1 != mode2:
-                    header.append('old mode %s\n' % mode1)
-                    header.append('new mode %s\n' % mode2)
+                    header.append('old mode %s' % mode1)
+                    header.append('new mode %s' % mode2)
                 if op is not None:
-                    header.append('%s from %s\n' % (op, path1))
-                    header.append('%s to %s\n' % (op, path2))
+                    header.append('%s from %s' % (op, path1))
+                    header.append('%s to %s' % (op, path2))
         elif revs and not repo.ui.quiet:
             header.append(diffline(path1, revs))
 
         if binarydiff and not opts.nobinary:
             text = mdiff.b85diff(content1, content2)
             if text and opts.git:
-                header.append('index %s..%s\n' %
+                header.append('index %s..%s' %
                               (gitindex(content1), gitindex(content2)))
         else:
             text = mdiff.unidiff(content1, date1,
                                  content2, date2,
                                  path1, path2, opts=opts)
         if header and (text or len(header) > 1):
-            yield ''.join(header)
+            yield '\n'.join(header) + '\n'
         if text:
             yield text