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

Mads Kiilerich mads at kiilerich.com
Thu Aug 26 17:44:51 CDT 2010


  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.

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

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

>   The -m/--mbox option is also very useful. Instead of previewing each
>   patchbomb message in a pager or sending the messages directly, it will
>   create a UNIX mailbox file with the patch emails. This mailbox file
> @@ -394,9 +398,17 @@
>       else:
>           msgs = getpatchmsgs(list(getpatches(revs)))
>
> +    showaddrs = []
> +
>       def getaddrs(opt, prpt=None, default=None):
>           addrs = opts.get(opt.replace('-', '_'))
> +        if opt != 'reply-to':
> +            showaddr = '%s:' % opt.capitalize()
> +        else:
> +            showaddr = 'Reply-To:'
> +
>           if addrs:
> +            showaddrs.append('%s %s' % (showaddr, ', '.join(addrs)))
>               return mail.addrlistencode(ui, addrs, _charsets,
>                                          opts.get('test'))
>
> @@ -404,6 +416,10 @@
>           if not addrs and prpt:
>               addrs = prompt(ui, prpt, default)
>
> +        if addrs:
> +            showaddr = '%s %s' % (showaddr, addrs)
> +        showaddrs.append(showaddr)
> +
>           return mail.addrlistencode(ui, [addrs], _charsets, opts.get('test'))
>
>       to = getaddrs('to', 'To')
> @@ -413,6 +429,19 @@
>
>       ui.write('\n')
>
> +    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.

> +            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"?

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

/Mads


More information about the Mercurial-devel mailing list