Patchwork D6462: commitextras: try to remove localrepo hacking to add extras to commit

login
register
mail settings
Submitter phabricator
Date May 30, 2019, 6:21 p.m.
Message ID <differential-rev-PHID-DREV-5wjby62lvpee7mo6t7kp-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/40278/
State New
Headers show

Comments

phabricator - May 30, 2019, 6:21 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/commitextras.py
  mercurial/commands.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - May 30, 2019, 6:21 p.m.
pulkit added a comment.


  I am not sure this is correct way to do this. But I will definitely like to remove the hacking around localrepo to add extras to a commit.

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - June 3, 2019, 9:32 p.m.
martinvonz added a comment.


  Can you elaborate a bit about what the current hack does and why it's bad?

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: martinvonz, mercurial-devel

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1669,7 +1669,10 @@ 
     branch = repo[None].branch()
     bheads = repo.branchheads(branch)
 
-    extra = {}
+    # extensions can wrap commands.commit and pass extras in opts
+    # 'extras' is used instead of 'extra' because the latter is taken by
+    # commitextras extension as a flag
+    extra = opts.get('extras', {})
     if opts.get('close_branch'):
         extra['close'] = '1'
 
diff --git a/hgext/commitextras.py b/hgext/commitextras.py
--- a/hgext/commitextras.py
+++ b/hgext/commitextras.py
@@ -17,7 +17,6 @@ 
     error,
     extensions,
     registrar,
-    util,
 )
 
 cmdtable = {}
@@ -38,35 +37,34 @@ 
 }
 
 def extsetup(ui):
-    entry = extensions.wrapcommand(commands.table, 'commit', _commit)
+    entry = extensions.wrapcommand(commands.table, 'commit', wrapcommit)
     options = entry[1]
     options.append(('', 'extra', [],
         _('set a changeset\'s extra values'), _("KEY=VALUE")))
 
-def _commit(orig, ui, repo, *pats, **opts):
-    if util.safehasattr(repo, 'unfiltered'):
-        repo = repo.unfiltered()
-    class repoextra(repo.__class__):
-        def commit(self, *innerpats, **inneropts):
-            extras = opts.get(r'extra')
-            for raw in extras:
-                if '=' not in raw:
-                    msg = _("unable to parse '%s', should follow "
-                            "KEY=VALUE format")
-                    raise error.Abort(msg % raw)
-                k, v = raw.split('=', 1)
-                if not k:
-                    msg = _("unable to parse '%s', keys can't be empty")
-                    raise error.Abort(msg % raw)
-                if re.search(br'[^\w-]', k):
-                    msg = _("keys can only contain ascii letters, digits,"
-                            " '_' and '-'")
-                    raise error.Abort(msg)
-                if k in usedinternally:
-                    msg = _("key '%s' is used internally, can't be set "
-                            "manually")
-                    raise error.Abort(msg % k)
-                inneropts[r'extra'][k] = v
-            return super(repoextra, self).commit(*innerpats, **inneropts)
-    repo.__class__ = repoextra
+def wrapcommit(orig, ui, repo, *pats, **opts):
+    extdict = opts.get(r'extras', {})
+    extras = opts.get(r'extra')
+    for raw in extras:
+        if '=' not in raw:
+            msg = _("unable to parse '%s', should follow "
+                    "KEY=VALUE format")
+            raise error.Abort(msg % raw)
+        k, v = raw.split('=', 1)
+        if not k:
+            msg = _("unable to parse '%s', keys can't be empty")
+            raise error.Abort(msg % raw)
+        if re.search(br'[^\w-]', k):
+            msg = _("keys can only contain ascii letters, digits,"
+                    " '_' and '-'")
+            raise error.Abort(msg)
+        if k in usedinternally:
+            msg = _("key '%s' is used internally, can't be set "
+                    "manually")
+            raise error.Abort(msg % k)
+        extdict[k] = v
+
+    # assign opts['extras'] to the dict before passing into
+    # commands.commit
+    opts['extras'] = extdict
     return orig(ui, repo, *pats, **opts)