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

[Kevin Bullock]

On 28 Jun 2013, at 4:06 PM, Ahmed S. Darwish wrote:

> 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 at 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/
> 
> 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')),

-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".

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

Our usual idiom here is to set the variable to None unconditionally, then check the condition. Eliminates an unnecessary 'else' clause.

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock