a3c22a4d68d7 — Steve Fink 3 years ago
Various prompting fixes, mixed in with other stuff (sorry).
2 files changed, 70 insertions(+), 59 deletions(-)

M __init__.py
M bzauth.py
M __init__.py +69 -58
@@ 124,7 124,7 @@ if util.safehasattr(registrar, 'configit
 class Abort(error.Abort):
     def __init__(self, *args, **kwargs):
         if args and isinstance(args[0], str):
-            args[0] = args[0].encode()
+            args = [args[0].encode(), args[1:]]
         return error.Abort.__init__(self, *args, **kwargs)
 
 

          
@@ 955,14 955,10 @@ def fill_values(values, ui, repo, api_se
     return values
 
 
-def update_patch(ui, repo, rev, bug, update_patch, interactive):
+def update_patch(ui, repo, rev, bug, update_patch, confirm):
     todo = []
     if update_patch:
         todo.append("description")
-    if todo:
-        if interactive and ui.prompt("Update patch " + " and ".join(todo) + " (y/n)?") != 'y':
-            ui.write(b"Exiting without updating patch\n")
-            return {rev: rev}
 
     ctx = repo[rev]
     msg = stringify(ctx.description()).splitlines()

          
@@ 978,35 974,41 @@ def update_patch(ui, repo, rev, bug, upd
         else:
             msg = ["Bug %s patch" % bug]
 
-    if changed and not ui.config(b'bzexport', b'dry-run', False):
-        newmessage = '\n'.join(msg).encode('utf-8')
-        if ctx == repo[b'.']:
-            opts = {
-                'amend': True,
-                'git': True,
-                'message': newmessage,
-                'logfile': None,
-            }
-            commands.commit(ui, repo, **opts)
-            return {rev: repo[b'.'].rev()}
-        else:
-            # Rewrite commit messages for patches deeper in the stack
-            # through the power of mozhg.
-            def makememctx(repo, ctx, revmap, copyfilectxfn):
-                msg = newmessage if repo[rev] == ctx else ctx.description()
-                parents = newparents(repo, ctx, revmap)
-                return context.memctx(repo, parents, msg,
-                                      ctx.files(), copyfilectxfn,
-                                      user=ctx.user(),
-                                      date=ctx.date(),
-                                      extra=dict(ctx.extra()))
+    if not todo or not changed or ui.config(b'bzexport', b'dry-run', False):
+        return {rev: rev}
+
+    if confirm and ui.prompt(("Update patch " + " and ".join(todo) + " (y/n)?").encode()) != b'y':
+        ui.write(b"Exiting without updating patch\n")
+        return {rev: rev}
 
-            todo = scmutil.revrange(repo, [("{}::".format(rev)).encode()])
-            todo = [repo[n].node() for n in todo]
-            nodemap = replacechangesets(repo, todo, makememctx,
-                                        backuptopic='addbugnum')
-            return {repo.unfiltered()[old].rev(): repo[new].rev()
-                    for old, new in nodemap.items()}
+    newmessage = '\n'.join(msg).encode('utf-8')
+    if ctx == repo[b'.']:
+        opts = {
+            'amend': True,
+            'git': True,
+            'message': newmessage,
+            'logfile': None,
+        }
+        commands.commit(ui, repo, **opts)
+        return {rev: repo[b'.'].rev()}
+    else:
+        # Rewrite commit messages for patches deeper in the stack
+        # through the power of mozhg.
+        def makememctx(repo, ctx, revmap, copyfilectxfn):
+            msg = newmessage if repo[rev] == ctx else ctx.description()
+            parents = newparents(repo, ctx, revmap)
+            return context.memctx(repo, parents, msg,
+                                  ctx.files(), copyfilectxfn,
+                                  user=ctx.user(),
+                                  date=ctx.date(),
+                                  extra=dict(ctx.extra()))
+
+        todo = scmutil.revrange(repo, [("{}::".format(rev)).encode()])
+        todo = [repo[n].node() for n in todo]
+        nodemap = replacechangesets(repo, todo, makememctx,
+                                    backuptopic='addbugnum')
+        return {repo.unfiltered()[old].rev(): repo[new].rev()
+                for old, new in nodemap.items()}
 
     return {rev: rev}
 

          
@@ 1158,11 1160,11 @@ def maybe_update_patch(ui, repo, rev, bu
     else:
         update = ui.configbool(b'bzexport', b'update-patch', False)
 
-    return update_patch(ui, repo, rev, bug, update, opts['interactive'])
+    return update_patch(ui, repo, rev, bug, update, opts['confirm'])
 
 def create_bug(ui, values, bzinfo, opts):
-    if opts['interactive'] and ui.prompt(_("Create bug in '%s' :: '%s' (y/n)?") %
-                                         (values['PRODUCT'], values['COMPONENT'])) != 'y':
+    if opts['confirm'] and ui.prompt(_("Create bug in '%s' :: '%s' (y/n)?") %
+                                     (values['PRODUCT'], values['COMPONENT'])) != 'y':
         ui.write(b"Exiting without creating bug\n")
         return
 

          
@@ 1229,7 1231,7 @@ def create_bug(ui, values, bzinfo, opts)
           b'List of users to CC on the bug (comma-separated search strings)'),
          (b'', b'new', False,
           b'Create a new bug'),
-         (b'i', b'interactive', False,
+         (b'i', b'confirm', False,
           b'Interactive -- request confirmation before any permanent action'),
          (b'', b'no-take-bug', False,
           b'Do not assign bug to myself'),

          
@@ 1325,7 1327,7 @@ for opt in newbug_opts:
           b'List of users to CC on the bug (comma-separated search strings)', B'USERS'),
          (b'', b'new', False,
           b'Create a new bug'),
-         (b'i', b'interactive', False,
+         (b'i', b'confirm', False,
           b'Interactive -- request confirmation before any permanent action'),
          (b'', b'no-take-bug', False,
           b'Do not assign bug to myself'),

          
@@ 1381,8 1383,9 @@ def phexport(ui, repo, *args, **opts):
                 if not ui.config(b'bzexport', b'dry-run', False):
                     bz.update_bug(auth, result['id'], params)
             except Exception as e:
-                import pdb; pdb.set_trace()
-                raise Abort(str(e))
+                ui.write_err(("Failed to take bug: " + str(e) + "\n").encode())
+                ui.write_err(("Request was PUT bug/%s:\n" % result['id']).encode())
+                ui.write_err(json.dumps(params, indent=4).encode())
 
 def parse_description(ui, desc, commit_message, bug, create_new_bug,
                       auto_review=False):

          
@@ 1535,7 1538,7 @@ def _bzexport_bugzilla(ui, repo, opts, r
     if filename == rev:
         filename = nodemap.get(rev, rev)
 
-    if opts['interactive'] and ui.prompt(_("Attach patch (y/n)?")) != 'y':
+    if opts['confirm'] and ui.prompt(_("Attach patch (y/n)?")) != 'y':
         ui.write(b"Exiting without creating attachment\n")
         return
 

          
@@ 1580,7 1583,7 @@ def _bzexport_bugzilla(ui, repo, opts, r
         print("%s uploaded as %s" % (rev, attachment_url))
 
     def pre_obsolete(**kwargs):
-        if not opts['interactive']:
+        if not opts['confirm']:
             return True
         url, filename, description = [kwargs[k] for k in ['url', 'filename', 'description']]
         return ui.prompt(_("Obsolete patch %s (%s) - %s (y/n)?") % (url, filename, description)) == 'y'

          
@@ 1813,14 1816,14 @@ def _bzexport_phabsend(ui, repo, opts, r
     revs = repo.anyrevs([rev.encode()], user=True)
     revs.sort()
 
-    if len(revs) > 1 and not opts['edit'] and not opts['same'] and not opts['first']:
-        raise Abort("Multiple revisions require --same, --first, or --edit")
+    if len(revs) > 1 and values['ATTACHCOMMENT'] != '<none>':
+        if not opts['edit'] and not opts['same'] and not opts['first']:
+            raise Abort("Multiple revisions require --same, --first, or --edit")
 
     if opts['new']:
         values = edit_form(ui, repo, values, 'new_bug_template')
         fill_values(values, ui, repo, api_server, finalize=True,
                     patchdata=contents)
-        import pdb; pdb.set_trace()
         bug = create_bug(ui, values, bzinfo, opts)
         values['BUGNUM'] = str(bug)
 

          
@@ 1886,16 1889,12 @@ def _bzexport_phabsend(ui, repo, opts, r
             firstline = repo[rev].description().split(b'\n')[0]
             ui.write(b'Patch description: ' + firstline + b'\n')
             bug = ui.prompt(msg.encode(), default=suggested_bug).decode()
-            values['BUGNUM'] = bug
+        values['BUGNUM'] = bug
 
         revmap.update(maybe_update_patch(ui, repo, rev, bug, opts))
     revs = reversed([recursive_lookup(r, revmap) for r in revs])
     opts.pop('rev', None)
 
-    if opts['interactive']:
-        del opts['interactive']
-        opts['confirm'] = True
-
     # If no reviewers are given, phabsend will parse them out of the commit
     # message.
     #

          
@@ 1916,27 1915,39 @@ def _bzexport_phabsend(ui, repo, opts, r
         assert recursive_lookup(tmp, revmap) == values['REVISION']
         opts['reviewer'] = []
         opts['blocker'] = []
-        for user in values['REVIEWERS']:
+        for user in values.get('REVIEWERS', []):
             if user.startswith("!"):
                 opts['blocker'].append(user[1:].encode('utf-8'))
             else:
                 opts['reviewer'].append(user.encode('utf-8'))
 
-        if values['ATTACHCOMMENT'] == '<none>':
+        # Ugh. Multiple patch comment handling is a complete mess. FIXME.
+
+        comment = values.get('ATTACHCOMMENT', '<none>')
+        if comment is None or comment == '<none>':
             desc = repo[values['REVISION']].description()
             if b'Differential Revision' in desc:
+                # If there is already a phabricator revision for this patch, do
+                # not repeat the comment extracted from the commit message.
                 opts['comment'] = 'Updated revision.'
             else:
                 _, opts['comment'] = extract_bug_num_and_desc(desc.decode())
         else:
-            opts['comment'] = values['ATTACHCOMMENT']
+            if 'Differential Revision' in comment:
+                opts['comment'] = 'Updated revision.'
+            else:
+                opts['comment'] = comment
+
         ui.write("{dryrun}submitting {REVISION} for bug {BUGNUM} to phabricator\n".format(
             **values,
             dryrun='not ' if dry_run else ''
         ).encode())
 
         if not dry_run:
-            phab.phabsend(ui, repo, values['REVISION'], **byteskwvals(opts))
+            if opts['confirm'] and ui.prompt(f"Phabsend {values['REVISION']} (y/n)?".encode()) != 'y':
+                ui.write(b"Skipping revision\n")
+            else:
+                phab.phabsend(ui, repo, values['REVISION'], **byteskwvals(opts))
 
     return bug
 

          
@@ 1944,7 1955,7 @@ def _bzexport_phabsend(ui, repo, opts, r
         (b'c', b'comment', b'', b'Comment to add with the bug'),
          (b'e', b'edit', False,
           b'Open a text editor to modify bug fields'),
-         (b'i', b'interactive', False,
+         (b'i', b'confirm', False,
           b'Interactive -- request confirmation before any permanent action'),
          (b'f', b'force', False,
           b'Proceed even if the working directory contains changes'),

          
@@ 2002,8 2013,8 @@ def newbug(ui, repo, *args, **opts):
         raise Abort("Invalid users")
     values['CC'] = select_users(cc, values['CC'])
 
-    if opts['interactive'] and ui.prompt(_("Create bug in '%s' :: '%s' (y/n)?") %
-                                         (values['PRODUCT'], values['COMPONENT'])) != 'y':
+    if opts['confirm'] and ui.prompt(_("Create bug in '%s' :: '%s' (y/n)?") %
+                                     (values['PRODUCT'], values['COMPONENT'])) != 'y':
         ui.write(b"Exiting without creating bug\n")
         return
 

          
M bzauth.py +1 -1
@@ 162,7 162,7 @@ class bzAuth:
 
         if 'error' in j:
             raise Exception('REST error on %s to %s: %s' % (
-                method, url, j['message']))
+                method, url, json.dumps(j)))
 
         return j