Adds undo command that changes working copy parent, bookmarks and
visible draft commits to a previous repo state.
Details
- Reviewers
quark - Group Reviewers
Restricted Project - Commits
- rFBHGX11039bbf64f1: undo: bare bones undo without tests
Diff Detail
- Repository
- rFBHGX Facebook Mercurial Extensions
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Event Timeline
hgext3rd/undo.py | ||
---|---|---|
36–39 | This is an easy mistake :-p, but: Importing inhibit will expose its functions. But whether inhibit is enabled or not, whether its uisetup runs or not is not guaranteed. If it's disabled (so its uisetup doesn't run), its API won't work. The correct way to do is to get the inhibit module from mercurial.extensions.find: from mercurial import ( extensions, ) try: inhibit = extensions.find('inhibit') except KeyError: raise error.Abort(_('undo requires inhibit to work properly')) else: inhibit.revive ... | |
293 | nit: remove the space before p. | |
306 | repo is guaranteed not None by Mercurial command handling so this if can be removed. | |
313 | Insert a blank line before this function. | |
325–332 | While commands.bookmark works, it's a high level API for running commands. In extension code, it's better to avoid calling other commands. repo._bookmarks has __setitem__ and __delitem__ (see bookmarks.py) which fit better in this case. Also search for recordchange for how to make transaction aware of the bookmark change. | |
338 | Similar to bookmarks, it's better to use lower level APIs since this is an hg extension. Things might be interesting are:
| |
341–342 | Nice use of existing revsets! | |
346 | repo.changectx(r) could be just repo[r]. The latter is more common. | |
353–359 | It is a bit strange that hidecommits only takes a single commit. And ui is not used in revealcommits. They could be made consistent: def hidecommits(repo, ctxs): .... def revealcommits(repo, ctxs): .... hidecommits(repo, repo.set(...)) revealcommits(repo, repo.set(...)) |
hgext3rd/undo.py | ||
---|---|---|
295 | Nit 'at least' |
Almost there.
hgext3rd/undo.py | ||
---|---|---|
325–329 | nit: could be simplified repo._bookmarks.clear(). | |
331 | nit: could be simplified to if mark: | |
361–363 | nit: need re-indent (press == in vim) | |
365–366 | Since the revision to revive might be hidden, I think you might want unfiltered repo here. nit: could be slightly simplified to ctxs = repo.unfiltered().set(rev) (ctxs is more common in hg codebase than contexts). scmutil.revrange is to resolve multiple revsets while there is just one single revset here. |
hgext3rd/undo.py | ||
---|---|---|
365–366 | The repo is unfiltered when this is called |
I see that's line 340. Might worth a comment on _undoto saying it expects an unfiltered repo.
This is an easy mistake :-p, but:
Importing inhibit will expose its functions. But whether inhibit is enabled or not, whether its uisetup runs or not is not guaranteed. If it's disabled (so its uisetup doesn't run), its API won't work.
The correct way to do is to get the inhibit module from mercurial.extensions.find: