Patchwork D6115: unamend: made match optional in firdirstate()

login
register
mail settings
Submitter phabricator
Date March 11, 2019, 3:59 p.m.
Message ID <differential-rev-PHID-DREV-yxzv6ik2be7jbh3is4nb-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39216/
State Superseded
Headers show

Comments

phabricator - March 11, 2019, 3:59 p.m.
taapas1128 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/D6115

AFFECTED FILES
  hgext/uncommit.py

CHANGE DETAILS




To: taapas1128, #hg-reviewers
Cc: mercurial-devel
phabricator - March 11, 2019, 4:14 p.m.
martinvonz added a comment.


  > unamend: made match optional in firdirstate()
  
  Typo'd "_fixdirstate"
  
  Also, the matcher has been optional since https://phab.mercurial-scm.org/D5661, so the description seems wrong. The commit message should also explain the reason for the patch, unless that's obvious (things like "remove unused argument 'foo' for bar()" don't need a motivation, IMO).

REPOSITORY
  rHG Mercurial

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

To: taapas1128, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - March 12, 2019, 4:58 p.m.
martinvonz added inline comments.

INLINE COMMENTS

> uncommit.py:184
>          curctx = repo['.']
> -
> +        match = scmutil.match(curctx, opts)
>          rewriteutil.precheck(repo, [curctx.rev()], 'unamend')

I don't think `hg unamend` accepts path arguments, so this matcher will always match everything, right?

REPOSITORY
  rHG Mercurial

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

To: taapas1128, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - March 12, 2019, 7:05 p.m.
taapas1128 marked an inline comment as done.
taapas1128 added inline comments.

INLINE COMMENTS

> martinvonz wrote in uncommit.py:184
> I don't think `hg unamend` accepts path arguments, so this matcher will always match everything, right?

sorry missed that went through the code of `scmutil.match` just now.

REPOSITORY
  rHG Mercurial

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

To: taapas1128, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - March 12, 2019, 10:33 p.m.
martinvonz added a comment.


  Could you  choose the "Abandon Revision" action on this patch so it's clear that it's not meant to be queued?

REPOSITORY
  rHG Mercurial

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

To: taapas1128, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - March 13, 2019, 12:49 a.m.
taapas1128 marked an inline comment as done.
taapas1128 added a comment.


  I have added '*pats' as argument for unamend() , won't it serve as an optimisation now ?

REPOSITORY
  rHG Mercurial

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

To: taapas1128, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - March 13, 2019, 12:56 a.m.
martinvonz added a comment.


  Does `hg unamend` accept patterns as arguments?

REPOSITORY
  rHG Mercurial

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

To: taapas1128, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - March 13, 2019, 4:50 a.m.
taapas1128 added a comment.


  Earlier it didn't but now it can I suppose it should after the modification. Is there supposed to be any other way to verify it ?(the tests pass successfully)

REPOSITORY
  rHG Mercurial

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

To: taapas1128, #hg-reviewers
Cc: martinvonz, mercurial-devel
phabricator - March 13, 2019, 4:56 a.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D6115#89208, @taapas1128 wrote:
  
  > Earlier it didn't but now it can I suppose it should after the modification.
  
  
  The `@command` decorator defines what arguments that command accepts. In this case, it's an empty list (on line 167), which means it accepts no arguments.
  
  > Is there supposed to be any other way to verify it ?
  
  You can simply run the command and see how it works. If you're in the root of the hg repo, just try this:
  
    $ ./hg unamend foo
    hg unamend: invalid arguments
    hg unamend
    
    undo the most recent amend operation on a current changeset
    
    (use 'hg unamend -h' to show more help)
  
  
  
  > (the tests pass successfully)
  
  I haven't checked, but I assume there are no tests for `hg unamend <file>` (or if there are, it still fails with the error I showed above).

REPOSITORY
  rHG Mercurial

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

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

Patch

diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -214,12 +214,13 @@ 
     marked as modified `hg status`)
     """
 
+    opts = pycompat.byteskwargs(opts)
     unfi = repo.unfiltered()
     with repo.wlock(), repo.lock(), repo.transaction('unamend'):
 
         # identify the commit from which to unamend
         curctx = repo['.']
-
+        match = scmutil.match(curctx, opts)
         rewriteutil.precheck(repo, [curctx.rev()], 'unamend')
 
         # identify the commit to which to unamend
@@ -256,7 +257,7 @@ 
         dirstate = repo.dirstate
 
         with dirstate.parentchange():
-            _fixdirstate(repo, curctx, newpredctx)
+            _fixdirstate(repo, curctx, newpredctx, match)
 
         mapping = {curctx.node(): (newprednode,)}
         scmutil.cleanupnodes(repo, mapping, 'unamend', fixphase=True)