[PATCH 4 of 4] ui: edit(): transplant: set HGREVISION environment variable for an editor

[Mads Kiilerich]

On 02/07/2014 05:26 AM, Alexander Drozdov wrote:
> # HG changeset patch
> # User Alexander Drozdov <al.drozdov at gmail.com>
> # Date 1391746185 -14400
> #      Fri Feb 07 08:09:45 2014 +0400
> # Node ID 8b9f2ec2dde1e08580bd5893ce6253acc940a837
> # Parent  01079ca49c16f7c0c3d53ba673f424368aea075e
> ui: edit(): transplant: set HGREVISION environment variable for an editor
>
> transplant command set 'transplant_source' extra for the revision.
> Allow an editor to access the extra using HGREVISION environment variable.

But ... it _is_ not this HGREVISION. Calling it that would be confusing. 
It is more like what we in revsets refer to as 'origin'.

I think it would be better to follow the HG_ naming convention used when 
Mercurial passes values to hooks. That would be inconsistent with HGUSER 
... but HGUSER is primarily input to Mercurial, not output...

>
> This may be useful when an editor is actually a script which modifies a commit
> message. Transplant filters is an alternative way to do it.

In my opinion this patch set is split up in so small pieces that the 
individual pieces don't make sense.

Why take an extra parameter if it never is used? How can it be correct 
to pass any value for this parameter when it not is used and we don't 
know anything about the type or the semantics?

In this case I would prefer it all folded into one patch. Others might 
like it the way it is ...

> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -8,6 +8,7 @@
>   from i18n import _
>   import errno, getpass, os, socket, sys, tempfile, traceback
>   import config, scmutil, util, error, formatter
> +from node import hex
>   
>   class ui(object):
>       def __init__(self, src=None):
> @@ -721,6 +722,10 @@
>               f.close()
>   
>               environ = {'HGUSER': user}
> +            for label in ('transplant_source',):
> +                if label in extra:
> +                    environ.update({'HGREVISION': hex(extra[label])})
> +                    break

Keep it simple. Why not just

+            if 'transplant_source' in extra:
+                environ.update({'HGREVISION': hex(extra['transplant_source'])})


/Mads