[PATCH 2 of 3] patchbomb: with --confirm display series details and ask for confirmation

[Christian Ebert]

* Mads Kiilerich on Friday, August 27, 2010 at 00:44:51 +0200
> Christian Ebert wrote, On 08/26/2010 05:24 PM:
>> # HG changeset patch
>> # User Christian Ebert<blacktrash at gmx.net>
>> # Date 1282836101 -3600
>> # Node ID 594b41c8ff5a81d99c4408a41f65172c9ea595e0
>> # Parent  5ef5d9a1d056449555c93dddb0e7887f21edf4b1
>> patchbomb: with --confirm display series details and ask for confirmation
>> 
>> The confirmation check summary looks like this:
>> 
>>  Patchbomb details:
>>  Subject: [patch 0 of x [flags]] subject of intro or single patch
>>  Subject: [patch 1 of x [flags]] subject
>>  [ ... ]
>>  From: sender
>>  To: recipient(s)
>>  Cc: if present or empty
>>  Bcc: if present or empty
>>  Reply-To: if present or empty
> 
> FWIW I would find it more natural if the addresses came before the subjects.

You're right. Leftover from previous version where the
confirmation was folded into other potential prompts.

>> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
>> --- a/hgext/patchbomb.py
>> +++ b/hgext/patchbomb.py
>> @@ -50,6 +50,10 @@
>> variable is set, your pager will be fired up once for each patchbomb
>> message, so you can verify everything is alright.
>> 
>> +Furthermore you can double-check the messages' details with the
>> +--confirm option. You will be prompted for an explicit sending
>> +confirmation.
> 
> Double-check? I would rather say that it is the first and only check.

Well, that's because I usually am prompted for Cc, and so it
always /feels/ like twice ...

> How about just:
> "With the --confirm option you will be prompted for confirmation
> before the messages are sent."

Sure. Fine with me.

[...]

>> +    if opts.get('confirm'):
>> +        ui.write(_('Patchbomb details:\n'))
>> +        for m, subj in msgs:
>> +            ui.write('Subject: %s\n' % subj)
>> +        ui.write('From: %s\n' % sender)
>> +        for addr in showaddrs:
>> +            ui.write('%s\n' % addr)
>> +        ans = prompt(ui, _('are you sure you want to send'), 'y', '?')
>> +        if not ans.lower().startswith('y'):
> 
> filemerge.py uses a sligtly different yes/no prompt. Let's do it the
> same way everywhere.

Hm. patchbomb itself has its own prompt function and:

def cdiffstat(ui, summary, patchlines):
    s = patch.diffstat(patchlines)
    if summary:
        ui.write(summary, '\n')
        ui.write(s, '\n')
    ans = prompt(ui, _('does the diffstat above look okay?'), 'y')
    if not ans.lower().startswith('y'):
        raise util.Abort(_('diffstat rejected'))
    return s

I tried to be consistent within patchbomb.

>> +            ui.write(_('patchbomb canceled\n'))
>> +            return
>> +        ui.write('\n')
>> +
>>     parent = opts.get('in_reply_to') or None
>>     # angle brackets may be omitted, they're not semantically part of the msg-id
>>     if parent is not None:
>> @@ -494,6 +523,7 @@
>>           ('i', 'inline', None, _('send patches as inline attachments')),
>>           ('', 'bcc', [], _('email addresses of blind carbon copy recipients')),
>>           ('c', 'cc', [], _('email addresses of copy recipients')),
>> +          ('', 'confirm', None, _('show summary and confirm before sending')),
> 
> "ask for confirmation"?

Ok. Omit "show summary"?

>>           ('d', 'diffstat', None, _('add diffstat output to messages')),
>>           ('', 'date', '', _('use the given date as the sending date')),
>>           ('', 'desc', '', _('use the given file as the series description')),
> 
> The tests should be included in the same patch.

Sure.

c
-- 
theatre - books - texts - movies
Black Trash Productions at home: http://www.blacktrash.org/
Black Trash Productions on Facebook:
http://www.facebook.com/blacktrashproductions