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

[Alexander Drozdov]

On 07.02.2014 19:51:03, Mads Kiilerich <mads at kiilerich.com> wrote:
> 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...
I've decided to use HGREVISION to allow users to move easier from transplant filters to 
graft. Transplant already sets HGREVISION for
--filter scripts. With HGREVISION, no changes in filters have to be done.

>>
>> 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 ...
Ok, I'll resend it all folded.
>> +            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'])})

Ok, I'll replace that with your code.