This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9911

errors: use InputError in uncommit extension
ClosedPublic

Authored by martinvonz on Jan 29 2021, 7:37 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG0e2becd1fe0c: errors: use InputError in uncommit extension

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Jan 29 2021, 7:37 PM
pulkit added a subscriber: pulkit.Jan 31 2021, 1:16 PM

I am on edge about whether these should be InputError or StateError.

mharbison72 added a subscriber: mharbison72.Jan 31 2021, 8:22 PM
In D9911#150313, @pulkit wrote:

I am on edge about whether these should be InputError or StateError.

I thought the heuristic was that InputErrors are always wrong (e.g. hg not-a-command not-a-file), and StateErrors mean the command is valid in some cases, but the state of the repo won't let it proceed. The 2 non-file checks here feel like state errors, given that. And the file checks feel like a weird combination of both (because some are for file tracking state, and one is whether the file exists at all).

martinvonz added a commit: rHG0e2becd1fe0c: errors: use InputError in uncommit extension.Feb 2 2021, 1:32 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG0e2becd1fe0c: errors: use InputError in uncommit extension (authored by martinvonz). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/uncommit.py (6 lines)
Mtests/test-unamend.t (2 lines)
Mtests/test-uncommit.t (10 lines)

Diff 25467

hgext/uncommit.py

if not allowdirtywcopy and (not pats or isdirtypath): if not allowdirtywcopy and (not pats or isdirtypath):
cmdutil.bailifchanged( cmdutil.bailifchanged(
repo, repo,
hint=_(b'requires --allow-dirty-working-copy to uncommit'), hint=_(b'requires --allow-dirty-working-copy to uncommit'),
) )
old = repo[b'.'] old = repo[b'.']
rewriteutil.precheck(repo, [old.rev()], b'uncommit') rewriteutil.precheck(repo, [old.rev()], b'uncommit')
if len(old.parents()) > 1: if len(old.parents()) > 1:
raise error.Abort(_(b"cannot uncommit merge changeset")) raise error.InputError(_(b"cannot uncommit merge changeset"))
match = scmutil.match(old, pats, opts) match = scmutil.match(old, pats, opts)
# Check all explicitly given files; abort if there's a problem. # Check all explicitly given files; abort if there's a problem.
if match.files(): if match.files():
s = old.status(old.p1(), match, listclean=True) s = old.status(old.p1(), match, listclean=True)
eligible = set(s.added) | set(s.modified) | set(s.removed) eligible = set(s.added) | set(s.modified) | set(s.removed)
hint = _( hint = _(
b"file was not changed in working directory parent" b"file was not changed in working directory parent"
) )
elif repo.wvfs.exists(f): elif repo.wvfs.exists(f):
hint = _(b"file was untracked in working directory parent") hint = _(b"file was untracked in working directory parent")
else: else:
hint = _(b"file does not exist") hint = _(b"file does not exist")
raise error.Abort( raise error.InputError(
_(b'cannot uncommit "%s"') % scmutil.getuipathfn(repo)(f), _(b'cannot uncommit "%s"') % scmutil.getuipathfn(repo)(f),
hint=hint, hint=hint,
) )
with repo.transaction(b'uncommit'): with repo.transaction(b'uncommit'):
if not (opts[b'message'] or opts[b'logfile']): if not (opts[b'message'] or opts[b'logfile']):
opts[b'message'] = old.description() opts[b'message'] = old.description()
message = cmdutil.logmessage(ui, opts) message = cmdutil.logmessage(ui, opts)
curctx = repo[b'.'] curctx = repo[b'.']
rewriteutil.precheck(repo, [curctx.rev()], b'unamend') rewriteutil.precheck(repo, [curctx.rev()], b'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 = _(b"changeset must have one predecessor, found %i predecessors") e = _(b"changeset must have one predecessor, found %i predecessors")
raise error.Abort(e % len(markers)) raise error.InputError(e % len(markers))
prednode = markers[0].prednode() prednode = markers[0].prednode()
predctx = unfi[prednode] predctx = unfi[prednode]
# add an extra so that we get a new hash # add an extra so that we get a new hash
# note: allowing unamend to undo an unamend is an intentional feature # note: allowing unamend to undo an unamend is an intentional feature
extras = predctx.extra() extras = predctx.extra()
extras[b'unamend_source'] = curctx.hex() extras[b'unamend_source'] = curctx.hex()

tests/test-unamend.t

o 1:29becc82797a Added b o 1:29becc82797a Added b
| |
o 0:18d04c59bb5d Added a o 0:18d04c59bb5d Added a
Trying to unamend when there was no amend done Trying to unamend when there was no amend done
$ hg unamend $ hg unamend
abort: changeset must have one predecessor, found 0 predecessors abort: changeset must have one predecessor, found 0 predecessors
[255] [10]
Unamend on clean wdir and tip Unamend on clean wdir and tip
$ echo "bar" >> h $ echo "bar" >> h
$ hg amend $ hg amend
$ hg exp $ hg exp
# HG changeset patch # HG changeset patch

tests/test-uncommit.t

$ hg status $ hg status
$ hg heads -T '{rev}:{node} {desc}' $ hg heads -T '{rev}:{node} {desc}'
5:0c07a3ccda771b25f1cb1edbd02e683723344ef1 new change abcde (no-eol) 5:0c07a3ccda771b25f1cb1edbd02e683723344ef1 new change abcde (no-eol)
Uncommit of non-existent and unchanged files aborts Uncommit of non-existent and unchanged files aborts
$ hg uncommit nothinghere $ hg uncommit nothinghere
abort: cannot uncommit "nothinghere" abort: cannot uncommit "nothinghere"
(file does not exist) (file does not exist)
[255] [10]
$ hg status $ hg status
$ hg uncommit file-abc $ hg uncommit file-abc
abort: cannot uncommit "file-abc" abort: cannot uncommit "file-abc"
(file was not changed in working directory parent) (file was not changed in working directory parent)
[255] [10]
$ hg status $ hg status
Try partial uncommit, also moves bookmark Try partial uncommit, also moves bookmark
$ hg bookmark $ hg bookmark
* foo 5:0c07a3ccda77 * foo 5:0c07a3ccda77
$ hg uncommit files $ hg uncommit files
$ hg status $ hg status
[20] [20]
$ hg status $ hg status
M a M a
$ hg commit -m 'merge a and b' $ hg commit -m 'merge a and b'
$ hg uncommit $ hg uncommit
abort: cannot uncommit merge changeset abort: cannot uncommit merge changeset
[255] [10]
$ hg status $ hg status
$ hg log -G -T '{rev}:{node} {desc}' --hidden $ hg log -G -T '{rev}:{node} {desc}' --hidden
@ 6:c03b9c37bc67bf504d4912061cfb527b47a63c6e merge a and b @ 6:c03b9c37bc67bf504d4912061cfb527b47a63c6e merge a and b
|\ |\
| o 5:2cd56cdde163ded2fbb16ba2f918c96046ab0bf2 b 3 | o 5:2cd56cdde163ded2fbb16ba2f918c96046ab0bf2 b 3
| | | |
| o 4:c3a0d5bb3b15834ffd2ef9ef603e93ec65cf2037 b 2 | o 4:c3a0d5bb3b15834ffd2ef9ef603e93ec65cf2037 b 2
... and errors out the same way when nothing can be uncommitted ... and errors out the same way when nothing can be uncommitted
$ hg rollback -q --config ui.rollback=True $ hg rollback -q --config ui.rollback=True
$ mkdir emptydir $ mkdir emptydir
$ hg uncommit emptydir $ hg uncommit emptydir
abort: cannot uncommit "emptydir" abort: cannot uncommit "emptydir"
(file was untracked in working directory parent) (file was untracked in working directory parent)
[255] [10]
$ cd emptydir $ cd emptydir
$ hg uncommit . $ hg uncommit .
abort: cannot uncommit "emptydir" abort: cannot uncommit "emptydir"
(file was untracked in working directory parent) (file was untracked in working directory parent)
[255] [10]
$ hg status $ hg status
$ cd .. $ cd ..