Patchwork [1,of,2] diff: add the --output option

login
register
mail settings
Submitter Ahmed S. Darwish
Date June 28, 2013, 9:06 p.m.
Message ID <20130628210631.GA7289@Darwish.PC>
Download mbox | patch
Permalink /patch/1758/
State Superseded
Headers show

Comments

Ahmed S. Darwish - June 28, 2013, 9:06 p.m.
Hi,

Thanks for reviewing my first mercurial patch :-), please check
the new one below with the corrections applied.

# HG changeset patch
# User Ahmed S. Darwish <darwish.07@gmail.com>
# Date 1372452887 -7200
# Node ID 5d24d9ab9035570951ac71481cccd4e58277c353
# Parent  84dc9669bd7173124d307f9fcac0632380de9ba6
diff: add the --output option

For all shells which cannot save a command standard output correctly,
this option is now introduced.

The most common example is Microsoft PowerShell, where the piped
output gets corrupted if saved using the standard, unix-like, stdout
rediction '>' operator. By transforming the piped output to a
different encoding, PowerShell saves 'hg diff' patch output to a
format __not understandable__ by GNU patch and 'hg patch' commands.

Windows PowerShell is installed by default on all Windows 7+
machines (Windows 7, 8, Server 2008, and Server 2012). An easily
invokable 'hg diff > temp.patch' command should thus be available
on these systems.

For a similar real-world scenario, please check:

http://www.webcitation.org/6Hiiqf425 - archived from the original
blog post at http://nbevans.wordpress.com/2011/02/22/lightweight-shelving-of-your-work-in-progress-with-mercurial/

exporting patch:
<fdopen>

--
Darwish
http://darwish.chasingpointers.com
Ahmed S. Darwish - June 28, 2013, 10:28 p.m.
Hi,

On Fri, Jun 28, 2013 at 11:06:31PM +0200, Ahmed S. Darwish wrote:
> diff: add the --output option
> 
> For all shells which cannot save a command standard output correctly,
> this option is now introduced.
> 
> The most common example is Microsoft PowerShell, where the piped
> output gets corrupted if saved using the standard, unix-like, stdout
> rediction '>' operator. By transforming the piped output to a
> different encoding, PowerShell saves 'hg diff' patch output to a
> format __not understandable__ by GNU patch and 'hg patch' commands.
> 
> Windows PowerShell is installed by default on all Windows 7+
> machines (Windows 7, 8, Server 2008, and Server 2012). An easily
> invokable 'hg diff > temp.patch' command should thus be available
> on these systems.
>

Would a patch that adds the above warning to `$ hg diff --help'
be acceptable?

Thanks,
 
--
Darwish
http://darwish.chasingpointers.com
Ahmed S. Darwish - June 30, 2013, 8:28 p.m.
Hi!

On Sat, Jun 29, 2013 at 02:17:35PM -0500, Kevin Bullock wrote:
> On 28 Jun 2013, at 4:06 PM, Ahmed S. Darwish wrote:
> >
> > --- a/mercurial/commands.py	Mon Jun 24 14:02:01 2013 -0400
> > +++ b/mercurial/commands.py	Fri Jun 28 22:54:47 2013 +0200
> > @@ -2676,7 +2676,9 @@
> >         ui.warn("%s\n" % res2)
> > 
> > @command('^diff',
> > -    [('r', 'rev', [], _('revision'), _('REV')),
> > +    [('o', 'output', '',
> > +     _('print output to file with formatted name'), _('FORMAT')),
> > +    ('r', 'rev', [], _('revision'), _('REV')),
> 
> -o/--output should probably come _after_ -c/--change in this case.
> We'll also need some documentation of the name format. This could
> either be a cross-reference to 'hg help export', or you could wrap
> the documentation in a ".. container:: verbose".
>

I guess several `hg export' FORMAT codes does not seem to make sense
here, espcially since I'm not proposing ``hg diff --output'' as a
replacement or alias for `hg export'.  I'll thus limit the FORMAT
characters to:

    "%%"  literal "%" character
    "%b"  basename of the exporting repository

while probably adding:

    "%d"  current time and date

and include the above three lines in `hg diff' own documentation.

> > 
> > +    if fname:
> > +        fp = cmdutil.makefileobj(repo, fname)
> > +    else:
> > +        fp = None
> 
> Our usual idiom here is to set the variable to None unconditionally,
> then check the condition. Eliminates an unnecessary 'else' clause.
> 

Will do.

Thanks for the review,

--
Darwish
http://darwish.chasingpointers.com

Patch

diff -r 84dc9669bd71 -r 5d24d9ab9035 mercurial/commands.py
--- a/mercurial/commands.py	Mon Jun 24 14:02:01 2013 -0400
+++ b/mercurial/commands.py	Fri Jun 28 22:54:47 2013 +0200
@@ -2676,7 +2676,9 @@ 
         ui.warn("%s\n" % res2)
 
 @command('^diff',
-    [('r', 'rev', [], _('revision'), _('REV')),
+    [('o', 'output', '',
+     _('print output to file with formatted name'), _('FORMAT')),
+    ('r', 'rev', [], _('revision'), _('REV')),
     ('c', 'change', '', _('change made by revision'), _('REV'))
     ] + diffopts + diffopts2 + walkopts + subrepoopts,
     _('[OPTION]... ([-c REV] | [-r REV1 [-r REV2]]) [FILE]...'))
@@ -2737,6 +2739,7 @@ 
     Returns 0 on success.
     """
 
+    fname = opts.get('output')
     revs = opts.get('rev')
     change = opts.get('change')
     stat = opts.get('stat')
@@ -2754,10 +2757,15 @@ 
     if reverse:
         node1, node2 = node2, node1
 
+    if fname:
+        fp = cmdutil.makefileobj(repo, fname)
+    else:
+        fp = None
+
     diffopts = patch.diffopts(ui, opts)
     m = scmutil.match(repo[node2], pats, opts)
     cmdutil.diffordiffstat(ui, repo, diffopts, node1, node2, m, stat=stat,
-                           listsubrepos=opts.get('subrepos'))
+                           fp=fp, listsubrepos=opts.get('subrepos'))
 
 @command('^export',
     [('o', 'output', '',