Patchwork D8150: histedit: py3 fixes for curses mode

login
register
mail settings
Submitter phabricator
Date Feb. 25, 2020, 9:32 p.m.
Message ID <differential-rev-PHID-DREV-dxibotzt6y4nchnkfkgj-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45318/
State Superseded
Headers show

Comments

phabricator - Feb. 25, 2020, 9:32 p.m.
sfink created this revision.
Herald added a reviewer: durin42.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

BRANCH
  stable

REVISION DETAIL
  https://phab.mercurial-scm.org/D8150

AFFECTED FILES
  hgext/histedit.py

CHANGE DETAILS




To: sfink, durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 25, 2020, 9:37 p.m.
sfink added a comment.


  Maybe this should be in the patch comment, but justification for the change:
  
  _chistedit is looking for bytestr opt keys, but is getting string keys. Normally, I'd use pycompat.byteskwargs at the top, except that _texthistedit is called at the bottom and it's expecting to see str keys initially (before it fixes them up). So I canonicalize in the caller (that chooses between an initial _chistedit or _texthistedit), except that it's passing the opts with **opts and Python requires keywords to be str. So I stop splatting the args and opts.
  
  I wasn't involved in the py3 port, but it does seem weird to me that strings are forced to be bytes everywhere. That makes sense for everything read out of a repo, but it seems like the command line should think in strings for both keys and non-path values. But I'm sure there were some hard decisions to make.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8150/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8150

To: sfink, durin42, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -1645,7 +1645,7 @@ 
             pass
 
 
-def _chistedit(ui, repo, *freeargs, **opts):
+def _chistedit(ui, repo, freeargs, opts):
     """interactively edit changeset history via a curses interface
 
     Provides a ncurses interface to histedit. Press ? in chistedit mode
@@ -1717,8 +1717,8 @@ 
             with repo.vfs(b'chistedit', b'w+') as fp:
                 for r in rules:
                     fp.write(r)
-                opts['commands'] = fp.name
-            return _texthistedit(ui, repo, *freeargs, **opts)
+                opts[b'commands'] = fp.name
+            return _texthistedit(ui, repo, freeargs, opts)
     except KeyboardInterrupt:
         pass
     return -1
@@ -1855,23 +1855,25 @@ 
     for intentional "edit" command, but also for resolving unexpected
     conflicts).
     """
+    opts = pycompat.byteskwargs(opts)
+
     # kludge: _chistedit only works for starting an edit, not aborting
     # or continuing, so fall back to regular _texthistedit for those
     # operations.
     if (
         ui.interface(b'histedit') == b'curses'
-        and _getgoal(pycompat.byteskwargs(opts)) == goalnew
+        and _getgoal(opts) == goalnew
     ):
-        return _chistedit(ui, repo, *freeargs, **opts)
-    return _texthistedit(ui, repo, *freeargs, **opts)
-
-
-def _texthistedit(ui, repo, *freeargs, **opts):
+        return _chistedit(ui, repo, freeargs, opts)
+    return _texthistedit(ui, repo, freeargs, opts)
+
+
+def _texthistedit(ui, repo, freeargs, opts):
     state = histeditstate(repo)
     with repo.wlock() as wlock, repo.lock() as lock:
         state.wlock = wlock
         state.lock = lock
-        _histedit(ui, repo, state, *freeargs, **opts)
+        _histedit(ui, repo, state, freeargs, opts)
 
 
 goalcontinue = b'continue'
@@ -1952,8 +1954,7 @@ 
                 )
 
 
-def _histedit(ui, repo, state, *freeargs, **opts):
-    opts = pycompat.byteskwargs(opts)
+def _histedit(ui, repo, state, freeargs, opts):
     fm = ui.formatter(b'histedit', opts)
     fm.startitem()
     goal = _getgoal(opts)