[PATCH] mq: include involved files in qrefresh -e constructed message (issue3647)

[Kevin Bullock]

On Dec 7, 2012, at 3:13 PM, Davide Bolcioni wrote:

> Kevin Bullock [kbullock+mercurial at ringworld.org] wrote
> 
>> On Dec 3, 2012, at 6:45 PM, Davide Bolcioni wrote:
>> 
>>> # HG changeset patch
>>> # User Davide Bolcioni <dbolcioni at fb.com> # Date 1354053904 28800 # 
>>> Node ID 85615e499caecbaf758363fd90ad52b42e118a1c
>>> # Parent  fb14a5dcdc62987512820531fe60719d650491b6
>>> mq: include involved files in qrefresh -e constructed message
>>> (issue3647)
> 
> [cut]
> 
>>> +        if not msg:
>>> +            msg.append(_('(replace this line with the message)'))
>> 
>> We don't fill in a placeholder message for commit, why should qrefresh 
>> -e be any different?
> 
> The MQ extension adds more stuff to juggle in your mind as part of the context during the workflow, so making this look different from a commit seems advisable to me in order to remind the user that the two visually similar operations actually are somewhat different. I am open to suggestions about the wording, e.g. "... with the patch refresh reason".

Mmmm, nope, not good enough.

1. Your placeholder text does nothing to remind the user that this is a qrefresh.

2. Adding a commit-message placeholder text isn't in line with your patch description.

3. The user might change their mind about editing the message; with your patch, they would (probably?) get a patch message set to the placeholder text.

4. In any case, we really don't do placeholder text AFAICT.

>>> +        msg.append(_("HG: Lines beginning with 'HG:' are 
>>> + removed."))
>>> +
>>> +        msg.extend([_('HG: added %s') % f for f in added])
>>> +        msg.extend([_('HG: changed %s') % f for f in modified])
>>> +        msg.extend([_('HG: removed %s') % f for f in removed])
>>> +        msg.extend([_('HG: deleted %s') % f for f in deleted])
>>> +
>>> +        message = ui.edit('\n'.join(msg), ph.user or ui.username())
>>>        # We don't want to lose the patch message if qrefresh fails
>>> (issue2062)
>>> +        msg = [t for t in message.split('\n') if not
>>> + t.startswith('HG: ')]
>> 
>> Ugh, extending the message just to split and filter it back out again?
> 
> The HG: lines are only there to help the user mentally lock on context. If I pass them to ui.edit(), they can come back and I do not want them in the patch message since they duplicate information which is in the patch already. Besides, promises are promises (cf. "Lines beginning with HG ...").

I just meant that you could save the undecorated message so that you don't have to remove the same 'HG:' lines you yourself added.

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