This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6115

unamend: added match as argument for scmutil.movedirstate
AbandonedPublic

Authored by taapas1128 on Mar 11 2019, 11:59 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

uncommit uses match as argument which serves the purpose of optimizer for _fixdirstate() . This adds a similar optimization for unamend.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

taapas1128 created this revision.Mar 11 2019, 11:59 AM
martinvonz added a subscriber: martinvonz.Mar 11 2019, 12:14 PM

unamend: made match optional in firdirstate()

Typo'd "_fixdirstate"

Also, the matcher has been optional since 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).

taapas1128 retitled this revision from unamend: made match optional in firdirstate() to unamend: added match as argument for _firdirstate() in unamend().Mar 11 2019, 1:22 PM
taapas1128 updated this revision to Diff 14465.
taapas1128 edited the summary of this revision. (Show Details)Mar 11 2019, 1:28 PM
taapas1128 retitled this revision from unamend: added match as argument for _firdirstate() in unamend() to unamend: added match as argument for scmutil.movedirstate.Mar 12 2019, 12:53 PM
taapas1128 updated this revision to Diff 14476.
taapas1128 edited the summary of this revision. (Show Details)Mar 12 2019, 12:55 PM
martinvonz added inline comments.Mar 12 2019, 12:58 PM
hgext/uncommit.py
184

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

taapas1128 edited the summary of this revision. (Show Details)Mar 12 2019, 3:03 PM
taapas1128 updated this revision to Diff 14478.
taapas1128 marked an inline comment as done.Mar 12 2019, 3:05 PM
taapas1128 added inline comments.
hgext/uncommit.py
184

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

martinvonz added a comment.Mar 12 2019, 6:33 PM

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

taapas1128 marked an inline comment as done.Mar 12 2019, 8:49 PM

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

martinvonz added a comment.Mar 12 2019, 8:56 PM

Does hg unamend accept patterns as arguments?

taapas1128 added a comment.Mar 13 2019, 12:50 AM

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)

martinvonz added a comment.Mar 13 2019, 12:56 AM
In 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).

taapas1128 abandoned this revision.Mar 13 2019, 3:28 AM

Revision Contents
Changeset List

PathPackages
Mhgext/uncommit.py (7 lines)

Diff 14478

hgext/uncommit.py

def predecessormarkers(ctx): def predecessormarkers(ctx):
"""yields the obsolete markers marking the given changeset as a successor""" """yields the obsolete markers marking the given changeset as a successor"""
for data in ctx.repo().obsstore.predecessors.get(ctx.node(), ()): for data in ctx.repo().obsstore.predecessors.get(ctx.node(), ()):
yield obsutil.marker(ctx.repo(), data) yield obsutil.marker(ctx.repo(), data)
@command('unamend', [], helpcategory=command.CATEGORY_CHANGE_MANAGEMENT, @command('unamend', [], helpcategory=command.CATEGORY_CHANGE_MANAGEMENT,
helpbasic=True) helpbasic=True)
def unamend(ui, repo, **opts): def unamend(ui, repo, *pats, **opts):
"""undo the most recent amend operation on a current changeset """undo the most recent amend operation on a current changeset
This command will roll back to the previous version of a changeset, This command will roll back to the previous version of a changeset,
leaving working directory in state in which it was before running leaving working directory in state in which it was before running
`hg amend` (e.g. files modified as part of an amend will be `hg amend` (e.g. files modified as part of an amend will be
marked as modified `hg status`) marked as modified `hg status`)
""" """
opts = pycompat.byteskwargs(opts)
unfi = repo.unfiltered() unfi = repo.unfiltered()
with repo.wlock(), repo.lock(), repo.transaction('unamend'): with repo.wlock(), repo.lock(), repo.transaction('unamend'):
# identify the commit from which to unamend # identify the commit from which to unamend
curctx = repo['.'] curctx = repo['.']
match = scmutil.match(curctx, pats, opts)
martinvonzUnsubmitted
Done

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

martinvonz: I don't think `hg unamend` accepts path arguments, so this matcher will always match everything…
taapas1128AuthorUnsubmitted
Not Done

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

taapas1128: sorry missed that went through the code of `scmutil.match` just now.
rewriteutil.precheck(repo, [curctx.rev()], 'unamend') rewriteutil.precheck(repo, [curctx.rev()], 'unamend')
# identify the commit to which to unamend # identify the commit to which to unamend
markers = list(predecessormarkers(curctx)) markers = list(predecessormarkers(curctx))
if len(markers) != 1: if len(markers) != 1:
e = _("changeset must have one predecessor, found %i predecessors") e = _("changeset must have one predecessor, found %i predecessors")
raise error.Abort(e % len(markers)) raise error.Abort(e % len(markers))
user=predctx.user(), user=predctx.user(),
date=predctx.date(), date=predctx.date(),
extra=extras) extra=extras)
newprednode = repo.commitctx(newctx) newprednode = repo.commitctx(newctx)
newpredctx = repo[newprednode] newpredctx = repo[newprednode]
dirstate = repo.dirstate dirstate = repo.dirstate
with dirstate.parentchange(): with dirstate.parentchange():
scmutil.movedirstate(repo, newpredctx) scmutil.movedirstate(repo, newpredctx, match)
mapping = {curctx.node(): (newprednode,)} mapping = {curctx.node(): (newprednode,)}
scmutil.cleanupnodes(repo, mapping, 'unamend', fixphase=True) scmutil.cleanupnodes(repo, mapping, 'unamend', fixphase=True)