patchbomb: make it easy for the user to decline sending an intro message.
authorGreg Ward <greg@gerg.ca>
Tue, 27 Sep 2011 22:38:47 -0400
changeset 15164 7bddec632821
parent 15162 d67a15b2e608
child 15165 3a55cee825ba
patchbomb: make it easy for the user to decline sending an intro message. - prompt(): respect interactive mode; clarify logic a bit - rename introneeded() to introwanted() and give it only one caller - add 'numbered' arg to makepatch() so it does not need to call introwanted() - factor makeintro() out of getpatchmsgs(), so it's easier to skip the intro message based on the user's behaviour Unexpected but perfectly reasonable side effect: in non-interactive mode, we don't show unanswerable "Cc" or "From" prompts anymore, so remove those from the test expectations.
hgext/patchbomb.py
tests/test-patchbomb.t
--- a/hgext/patchbomb.py	Mon Sep 26 21:29:13 2011 -0400
+++ b/hgext/patchbomb.py	Tue Sep 27 22:38:47 2011 -0400
@@ -57,24 +57,25 @@
 command = cmdutil.command(cmdtable)
 
 def prompt(ui, prompt, default=None, rest=':'):
-    if not ui.interactive() and default is None:
-        raise util.Abort(_("%s Please enter a valid value" % (prompt + rest)))
+    if not ui.interactive():
+        return default
     if default:
         prompt += ' [%s]' % default
     prompt += rest
     while True:
-        r = ui.prompt(prompt, default=default)
-        if r:
-            return r
-        if default is not None:
+        result = ui.prompt(prompt, default=default)
+        if result is not None:
+            return result
+        elif default is not None:
             return default
-        ui.warn(_('Please enter a valid value.\n'))
+        else:
+            ui.warn(_('Please enter a valid value.\n'))
 
-def introneeded(opts, number):
-    '''is an introductory message required?'''
+def introwanted(opts, number):
+    '''is an introductory message apparently wanted?'''
     return number > 1 or opts.get('intro') or opts.get('desc')
 
-def makepatch(ui, repo, patchlines, opts, _charsets, idx, total,
+def makepatch(ui, repo, patchlines, opts, _charsets, idx, total, numbered,
               patchname=None):
 
     desc = []
@@ -141,7 +142,7 @@
         flag = ' ' + flag
 
     subj = desc[0].strip().rstrip('. ')
-    if not introneeded(opts, total):
+    if not numbered:
         subj = '[PATCH%s] %s' % (flag, opts.get('subject') or subj)
     else:
         tlen = len(str(total))
@@ -352,51 +353,66 @@
             ui.write(_('\nWrite the introductory message for the '
                        'patch series.\n\n'))
             body = ui.edit(body, sender)
-            # Save serie description in case sendmail fails
+            # Save series description in case sendmail fails
             msgfile = repo.opener('last-email.txt', 'wb')
             msgfile.write(body)
             msgfile.close()
         return body
 
     def getpatchmsgs(patches, patchnames=None):
-        jumbo = []
         msgs = []
 
         ui.write(_('This patch series consists of %d patches.\n\n')
                  % len(patches))
 
+        # build the intro message, or skip it if the user declines
+        if introwanted(opts, len(patches)):
+            msg = makeintro(patches)
+            if msg:
+                msgs.append(msg)
+
+        # are we going to send more than one message?
+        numbered = len(msgs) + len(patches) > 1
+
+        # now generate the actual patch messages
         name = None
         for i, p in enumerate(patches):
-            jumbo.extend(p)
             if patchnames:
                 name = patchnames[i]
             msg = makepatch(ui, repo, p, opts, _charsets, i + 1,
-                            len(patches), name)
+                            len(patches), numbered, name)
             msgs.append(msg)
 
-        if introneeded(opts, len(patches)):
-            tlen = len(str(len(patches)))
+        return msgs
+
+    def makeintro(patches):
+        tlen = len(str(len(patches)))
 
-            flag = ' '.join(opts.get('flag'))
-            if flag:
-                subj = '[PATCH %0*d of %d %s]' % (tlen, 0, len(patches), flag)
-            else:
-                subj = '[PATCH %0*d of %d]' % (tlen, 0, len(patches))
-            subj += ' ' + (opts.get('subject') or
-                           prompt(ui, 'Subject: ', rest=subj))
+        flag = opts.get('flag') or ''
+        if flag:
+            flag = ' ' + ' '.join(flag)
+        prefix = '[PATCH %0*d of %d%s]' % (tlen, 0, len(patches), flag)
+
+        subj = (opts.get('subject') or
+                prompt(ui, 'Subject: ', rest=prefix, default=''))
+        if not subj:
+            return None         # skip intro if the user doesn't bother
 
-            body = ''
-            ds = patch.diffstat(jumbo)
-            if ds and opts.get('diffstat'):
-                body = '\n' + ds
+        subj = prefix + ' ' + subj
 
-            body = getdescription(body, sender)
-            msg = mail.mimeencode(ui, body, _charsets, opts.get('test'))
-            msg['Subject'] = mail.headencode(ui, subj, _charsets,
-                                             opts.get('test'))
+        body = ''
+        if opts.get('diffstat'):
+            # generate a cumulative diffstat of the whole patch series
+            diffstat = patch.diffstat(sum(patches, []))
+            body = '\n' + diffstat
+        else:
+            diffstat = None
 
-            msgs.insert(0, (msg, subj, ds))
-        return msgs
+        body = getdescription(body, sender)
+        msg = mail.mimeencode(ui, body, _charsets, opts.get('test'))
+        msg['Subject'] = mail.headencode(ui, subj, _charsets,
+                                         opts.get('test'))
+        return (msg, subj, diffstat)
 
     def getbundlemsgs(bundle):
         subj = (opts.get('subject')
@@ -442,14 +458,19 @@
                 ui.config('patchbomb', configkey) or
                 '')
         if not addr and ask:
-            addr = prompt(ui, header, default)
+            addr = prompt(ui, header, default=default)
         if addr:
             showaddrs.append('%s: %s' % (header, addr))
-        return mail.addrlistencode(ui, [addr], _charsets, opts.get('test'))
+            return mail.addrlistencode(ui, [addr], _charsets, opts.get('test'))
+        else:
+            return default
 
     to = getaddrs('To', ask=True)
-    cc = getaddrs('Cc', ask=True, default='')
-    bcc = getaddrs('Bcc')
+    if not to:
+        # we can get here in non-interactive mode
+        raise util.Abort(_('no recipient addresses provided'))
+    cc = getaddrs('Cc', ask=True, default='') or []
+    bcc = getaddrs('Bcc') or []
     replyto = getaddrs('Reply-To')
 
     if opts.get('diffstat') or opts.get('confirm'):
--- a/tests/test-patchbomb.t	Mon Sep 26 21:29:13 2011 -0400
+++ b/tests/test-patchbomb.t	Tue Sep 27 22:38:47 2011 -0400
@@ -1469,13 +1469,70 @@
   +ff2c9fa2018b15fa74b33363bda9527323e2a99f two
   +ff2c9fa2018b15fa74b33363bda9527323e2a99f two.diff
   
-
+no intro message in non-interactive mode
   $ hg email --date '1970-1-1 0:1' -n -f quux -t foo -c bar --in-reply-to baz \
-  >  -r 0:1
+  >  -r 0:1 | fixheaders
   This patch series consists of 2 patches.
   
-  abort: Subject: [PATCH 0 of 2] Please enter a valid value
-  [255]
+  
+  Displaying [PATCH 1 of 2] a ...
+  Content-Type: text/plain; charset="us-ascii"
+  MIME-Version: 1.0
+  Content-Transfer-Encoding: 7bit
+  Subject: [PATCH 1 of 2] a
+  X-Mercurial-Node: 8580ff50825a50c8f716709acdf8de0deddcd6ab
+  Message-Id: <8580ff50825a50c8f716.60@
+  In-Reply-To: <baz>
+  References: <baz>
+  User-Agent: Mercurial-patchbomb
+  Date: Thu, 01 Jan 1970 00:01:00 +0000
+  From: quux
+  To: foo
+  Cc: bar
+  
+  # HG changeset patch
+  # User test
+  # Date 1 0
+  # Node ID 8580ff50825a50c8f716709acdf8de0deddcd6ab
+  # Parent  0000000000000000000000000000000000000000
+  a
+  
+  diff -r 000000000000 -r 8580ff50825a a
+  --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+  +++ b/a	Thu Jan 01 00:00:01 1970 +0000
+  @@ -0,0 +1,1 @@
+  +a
+  
+  Displaying [PATCH 2 of 2] b ...
+  Content-Type: text/plain; charset="us-ascii"
+  MIME-Version: 1.0
+  Content-Transfer-Encoding: 7bit
+  Subject: [PATCH 2 of 2] b
+  X-Mercurial-Node: 97d72e5f12c7e84f85064aa72e5a297142c36ed9
+  Message-Id: <97d72e5f12c7e84f8506.61@
+  In-Reply-To: <8580ff50825a50c8f716.60@
+  References: <8580ff50825a50c8f716.60@
+  User-Agent: Mercurial-patchbomb
+  Date: Thu, 01 Jan 1970 00:01:01 +0000
+  From: quux
+  To: foo
+  Cc: bar
+  
+  # HG changeset patch
+  # User test
+  # Date 2 0
+  # Node ID 97d72e5f12c7e84f85064aa72e5a297142c36ed9
+  # Parent  8580ff50825a50c8f716709acdf8de0deddcd6ab
+  b
+  
+  diff -r 8580ff50825a -r 97d72e5f12c7 b
+  --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+  +++ b/b	Thu Jan 01 00:00:02 1970 +0000
+  @@ -0,0 +1,1 @@
+  +b
+  
+
+
 
   $ hg email --date '1970-1-1 0:1' -n -f quux -t foo -c bar --in-reply-to baz \
   >  -s test -r 0:1 | fixheaders
@@ -1826,7 +1883,6 @@
   $ hg email --date '1980-1-1 0:1' -m tmp.mbox -f quux -t "bar@${UUML}nicode.com" -s test -r 0
   This patch series consists of 1 patches.
   
-  Cc: 
   
   Writing [PATCH] test ...
 
@@ -1871,13 +1927,11 @@
   $ hg email --date '1980-1-1 0:1' -n -t foo -s test -o ../t
   comparing with ../t
   searching for changes
-  From [test]: test
   This patch series consists of 8 patches.
   
   
   Write the introductory message for the patch series.
   
-  Cc: 
   
   Displaying [PATCH 0 of 8] test ...
   Content-Type: text/plain; charset="us-ascii"
@@ -2145,10 +2199,8 @@
   $ hg email --date '1980-1-1 0:1' -n -t foo -s test -o ../t#test
   comparing with ../t
   searching for changes
-  From [test]: test
   This patch series consists of 1 patches.
   
-  Cc: 
   
   Displaying [PATCH] test ...
   Content-Type: text/plain; charset="us-ascii"