| 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
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
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
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
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
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
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
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
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)