This is an archive of the discontinued Mercurial Phabricator instance.

undo: basic --preview
ClosedPublic

Authored by felixmerk on Aug 1 2017, 6:46 PM.

Download Raw Diff

Details

Reviewers

stash
quark

Group Reviewers

Restricted Project

Commits

rFBHGX883372ac2468: undo: basic --preview

Summary

Allows for hg undo --preview to see smartlog like preview of what undo will do.
This will be neccesery before a general roll-out so users can see what undo will
do. This first iteration only shows what "hg undo" will do, not redo and not
--index.

Diff Detail

Repository

rFBHGX Facebook Mercurial Extensions

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

felixmerk created this revision.Aug 1 2017, 6:46 PM

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 1 2017, 6:46 PM

See screen shot for what it actually looks like.

So in current implementation hg undo -p prints combined smartlog output of current state and state after undo, right?
I imagined that hg undo --preview would just show only state after undo, but your approach seems better.

As for the tests:
I've added

> [templatealias]
> undopreview = '{if(undonecommits, "Undone")}'
> EOF

to the $HGRCPATH and then ran

  $ hg undo -p 

+  @  Undone
+  |
+  o
+  |
+  o
+  |
+  o
+  |
+  o
+  |
+  | o
+  |/
+  o
+  |
+  o
+  |
+  | o
+  |/
+  o
+  |
+  o
+

So this is basically what you've already done but with actually testing hg undo -p, not hg log.
Using this you can test all of your templates.

hgext3rd/undo.py
446–447	BTW, in https://phab.mercurial-scm.org/D184 @quark suggested to use 'if{revset...}' instead of templates. Have you considered it? I'm not saying this is necessarily better, just want to make sure you've considered this option too.

This revision now requires changes to proceed.Aug 3 2017, 6:27 AM

Thats a creative way to test this. Awesome, thank you!

hgext3rd/undo.py
446–447	That seems like a cool approach. I hadn't considered it, I'll look into what the pros and cons are.

I think undo --preview should not depend on smartlog. So users do not need smartlog enabled to use undo preview.

hg smartlog is basically (although there are some minor differences):

hg log -G -T '{sl}' -r 'ancestor(master, draft())+draft()'

What we need here are:

a revset selecting commits to show
a template to beautify the output

Thinking about external users, the best practice is to make the above config options (ex. undo.preview.revset, undo.preview.template) and provide a minimal default.

We can define local (only available in undo command) revsets (see 4672db164) like UNDODEL, UNDOADD which contains commits to hide and unhide. A default undo.preview.revset could be UNDODEL + UNDOADD + ancestor(UNDODEL+UNDOADD). Internally we can customize it so it also includes remote/master.

The template could be something like {if(revset("{rev} and UNDODEL"), "-")}{if(revset("{rev} and UNDOADD"), "+")} {node|shortest} {author|emailuser} {desc|firstline} {bookmarks...}. It does not need to have colors. Internally we can customize it to fit the smartlog style.

hgext3rd/undo.py
416	This has issues: `repo` is mutable so the cache is not guaranteed to be always correct `repo` object will not be freed because it gets reference by this decorator
905	This is a no-op. You might want `repo = repo.filtered('visible')`. With the change I proposed, it seems we don't need to change the filter here.

quark mentioned this in D207: undo: add --index and redo support to preview.Aug 4 2017, 1:44 PM

felixmerk edited edge metadata.Aug 7 2017, 3:32 PM

felixmerk updated this revision to Diff 609.

Are you going to address comments about {if(revset("{rev} and UNDODEL"), "-")} ?

tests/test-undo.t
709–712	Don't need it anymore?
734–737	Same here

Are you going to also show working copy changes?

hgext3rd/undo.py
427	With D207 this comment is not correct - it's not necessarily just last command.
446	Same here - not necessarily one repo state ago

{if(revset("{rev} and UNDODEL"), "-")} type stuff is on the facebook.rc side, so there isn't much to address about it here. The revsets/templates to do it exist and I've tested it.

felixmerk added inline comments.Aug 8 2017, 1:26 PM

hgext3rd/undo.py
427	It's still the (relative) last command. In 207 we just lie about what the last command is. Which really means I should fix 207.

stash accepted this revision.Aug 9 2017, 10:57 AM

Be sure to remove smartlog config before landing.

hgext3rd/undo.py
457	I see this is a global state on filesystem. Per discussion we could make it better - move the global state from filesystem to `ui`, as demostrated by P57852191. Let's do that in a follow-up.
892	I didn't see the definition of `undopreview` template. Maybe provide a fallback in the code. That could be a follow up.

This revision is now accepted and ready to land.Aug 9 2017, 6:29 PM

felixmerk updated this revision to Diff 720.Aug 9 2017, 7:52 PM

Closed by commit rFBHGX883372ac2468: undo: basic --preview (authored by felixmerk). · Explain WhyAug 14 2017, 5:19 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			hgext3rd/undo.py (124 lines)
M			tests/test-undo.t (27 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	489		Aug 1 2017, 6:46 PM	★	★
Diff 2	609		Aug 7 2017, 3:32 PM	★	★
Diff 3	720		Aug 9 2017, 7:52 PM	★	★
Diff 4	881	rFBHGX883372ac24686786318a77193ae0e98e917f39ec	Aug 11 2017, 9:05 PM	★	★

Diff 881

hgext3rd/undo.py

	error,			error,
	extensions,			extensions,
	hg,			hg,
	localrepo,			localrepo,
	lock as lockmod,			lock as lockmod,
	obsolete,			obsolete,
	obsutil,			obsutil,
	phases,			phases,
				pycompat,
	registrar,			registrar,
	revlog,			revlog,
	revset,			revset,
	revsetlang,			revsetlang,
	smartset,			smartset,
				templatekw,
	transaction,			transaction,
	util,			util,
	)			)

	from mercurial.node import (			from mercurial.node import (
	bin,			bin,
	hex,			hex,
	nullid,			nullid,
	# undolog specific lock			# undolog specific lock
	# allows running command during other commands when			# allows running command during other commands when
	# otherwise legal. Could cause weird undolog states,			# otherwise legal. Could cause weird undolog states,
	# which gap handling generally covers.			# which gap handling generally covers.
	try:			try:
	try:			try:
	repo.vfs.makedirs('undolog')			repo.vfs.makedirs('undolog')
	except OSError:			except OSError:
	repo.ui.debug("can't make undolog folder in .hg")			repo.ui.debug("can't make undolog folder in .hg\n")
	return changes			return changes
	with lockmod.lock(repo.vfs, "undolog/lock", desc="undolog",			with lockmod.lock(repo.vfs, "undolog/lock", desc="undolog",
	timeout=2):			timeout=2):
	# developer config: undo._duringundologlock			# developer config: undo._duringundologlock
	if repo.ui.configbool('undo', '_duringundologlock'):			if repo.ui.configbool('undo', '_duringundologlock'):
	repo.hook("duringundologlock")			repo.hook("duringundologlock")
	tr = lighttransaction(repo)			tr = lighttransaction(repo)
	with tr:			with tr:
	False if no changes to record			False if no changes to record
	'''			'''
	newnodes = {			newnodes = {
	'bookmarks': _logbookmarks(repo, tr),			'bookmarks': _logbookmarks(repo, tr),
	'draftheads': _logdraftheads(repo, tr),			'draftheads': _logdraftheads(repo, tr),
	'workingparent': _logworkingparent(repo, tr),			'workingparent': _logworkingparent(repo, tr),
	}			}
	try:			try:
	exsistingnodes = _readindex(repo, 0)			existingnodes = _readindex(repo, 0)
	except IndexError:			except IndexError:
	exsistingnodes = {}			existingnodes = {}
	if all(newnodes.get(x) == exsistingnodes.get(x) for x in newnodes.keys()):			if all(newnodes.get(x) == existingnodes.get(x) for x in newnodes.keys()):
	# no changes to record			# no changes to record
	return False			return False
	else:			else:
	newnodes.update({			newnodes.update({
	'date': _logdate(repo, tr),			'date': _logdate(repo, tr),
	'command': _logcommand(repo, tr, command),			'command': _logcommand(repo, tr, command),
	})			})
	_logindex(repo, tr, newnodes)			_logindex(repo, tr, newnodes)
	# we assume that there is only a single rev			# we assume that there is only a single rev
	if repo[revs.first()].phase() == phases.public:			if repo[revs.first()].phase() == phases.public:
	querystring = revsetlang.formatspec('(children(%d) & draft())::',			querystring = revsetlang.formatspec('(children(%d) & draft())::',
	revs.first())			revs.first())
	else:			else:
	querystring = revsetlang.formatspec('((::%ld) & draft())::', revs)			querystring = revsetlang.formatspec('((::%ld) & draft())::', revs)
	return subset & smartset.baseset(repo.revs(querystring))			return subset & smartset.baseset(repo.revs(querystring))

				# Templates
				keywords = {}

				templatekeyword = registrar.templatekeyword(keywords)

				def _undonehexnodes(repo):
				quarkUnsubmitted Not Done This has issues: `repo` is mutable so the cache is not guaranteed to be always correct `repo` object will not be freed because it gets reference by this decorator quark: This has issues: - `repo` is mutable so the cache is not guaranteed to be always correct…
				repo = repo.unfiltered()
				reverseindex = _computerelative(repo, 1)
				revstring = revsetlang.formatspec('draft() - olddraft(%d)', reverseindex)
				revs = repo.revs(revstring)
				tonode = repo.changelog.node
				hexnodes = [repo[tonode(x)] for x in revs]
				return hexnodes

				@templatekeyword("undonecommits")
				def showundonecommits(repo, ctx, **args):
				"""String. Changectxs added by last command."""
				stashUnsubmitted Not Done With D207 this comment is not correct - it's not necessarily just last command. stash: With D207 this comment is not correct - it's not necessarily just last command.
				felixmerkAuthorUnsubmitted Not Done It's still the (relative) last command. In 207 we just lie about what the last command is. Which really means I should fix 207. felixmerk: It's still the (relative) last command. In 207 we just lie about what the last command is.
				hexnodes = _undonehexnodes(repo)
				if ctx in hexnodes:
				result = ctx.hex()
				else:
				result = None
				return result

				def _donehexnodes(repo):
				repo = repo.unfiltered()
				reverseindex = _computerelative(repo, 1)
				revstring = revsetlang.formatspec('olddraft(%d)', reverseindex)
				revs = repo.revs(revstring)
				tonode = repo.changelog.node
				hexnodes = [repo[tonode(x)] for x in revs]
				return hexnodes

				@templatekeyword("donecommits")
				def showdonecommits(repo, ctx, **args):
				"""String. Changectxs one repo state ago."""
				stashUnsubmitted Not Done Same here - not necessarily one repo state ago stash: Same here - not necessarily one repo state ago
				hexnodes = _donehexnodes(repo)
				stashUnsubmitted Not Done BTW, in https://phab.mercurial-scm.org/D184 @quark suggested to use 'if{revset...}' instead of templates. Have you considered it? I'm not saying this is necessarily better, just want to make sure you've considered this option too. stash: BTW, in https://phab.mercurial-scm.org/D184 @quark suggested to use 'if{revset...}' instead of…
				felixmerkAuthorUnsubmitted Not Done That seems like a cool approach. I hadn't considered it, I'll look into what the pros and cons are. felixmerk: That seems like a cool approach. I hadn't considered it, I'll look into what the pros and cons…
				if ctx in hexnodes:
				result = ctx.hex()
				else:
				result = None
				return result

				def _oldmarks(repo):
				reverseindex = _computerelative(repo, 1)
				nodedict = _readindex(repo, reverseindex)
				bookstring = _readnode(repo, "bookmarks.i", nodedict["bookmarks"])
				quarkUnsubmitted Not Done I see this is a global state on filesystem. Per discussion we could make it better - move the global state from filesystem to `ui`, as demostrated by P57852191. Let's do that in a follow-up. quark: I see this is a global state on filesystem. Per discussion we could make it better - move the…
				oldmarks = bookstring.split("\n")
				result = []
				for mark in oldmarks:
				kv = mark.rsplit(" ", 1)
				if len(kv) == 2:
				result.append(kv)
				return result

				@templatekeyword("oldbookmarks")
				def showoldbookmarks(**args):
				"""List of strings. Bookmarks one repo state ago."""
				args = pycompat.byteskwargs(args)
				repo = args['ctx']._repo
				oldmarks = _oldmarks(repo)
				bookmarks = []
				for kv in oldmarks:
				if repo[kv[1]] == repo[args['ctx']]:
				bookmarks.append(kv[0])
				active = repo._activebookmark
				makemap = lambda v: {'bookmark': v, 'active': active, 'current': active}
				f = templatekw._showlist('bookmark', bookmarks, args)
				return templatekw._hybrid(f, bookmarks, makemap, lambda x: x['bookmark'])

				@templatekeyword("removedbookmarks")
				def showremovedbookmarks(**args):
				"""List of strings. Bookmarks added/moved by last command."""
				args = pycompat.byteskwargs(args)
				repo = args['ctx']._repo
				currentbookmarks = args['ctx'].bookmarks()
				oldmarks = _oldmarks(repo)
				oldbookmarks = []
				for kv in oldmarks:
				if repo[kv[1]] == repo[args['ctx']]:
				oldbookmarks.append(kv[0])
				bookmarks = list(set(currentbookmarks) - set(oldbookmarks))
				active = repo._activebookmark
				makemap = lambda v: {'bookmark': v, 'active': active, 'current': active}
				f = templatekw._showlist('bookmark', bookmarks, args)
				return templatekw._hybrid(f, bookmarks, makemap, lambda x: x['bookmark'])

	# Undo:			# Undo:

	@command('undo', [			@command('undo', [
	('a', 'absolute', False, _("absolute based on command index instead of "			('a', 'absolute', False, _("absolute based on command index instead of "
	"relative undo")),			"relative undo")),
	('b', 'branch', "", _("local branch undo, accepts commit hash "			('b', 'branch', "", _("local branch undo, accepts commit hash "
	"(ADVANCED)")),			"(ADVANCED)")),
	('f', 'force', False, _("undo across missing undo history (ADVANCED)")),			('f', 'force', False, _("undo across missing undo history (ADVANCED)")),
	('k', 'keep', False, _("keep working copy changes")),			('k', 'keep', False, _("keep working copy changes")),
	('n', 'index', 1, _("how many steps to undo back")),			('n', 'index', 1, _("how many steps to undo back")),
				('p', 'preview', False, _("see smartlog like preview of future undo "
				"state")),
	])			])
	def undo(ui, repo, args, *opts):			def undo(ui, repo, args, *opts):
	"""perform an undo			"""perform an undo

	Undoes an undoable command. An undoable command is one that changed at			Undoes an undoable command. An undoable command is one that changed at
	least one of the following three: bookmarks, working copy parent or			least one of the following three: bookmarks, working copy parent or
	changesets. Note that this specifically does not include commands like log.			changesets. Note that this specifically does not include commands like log.
	It will include update if update changes the working copy parent (you update			It will include update if update changes the working copy parent (you update
	debugundohistory and specifically jumped to using undo --index --absolute.			debugundohistory and specifically jumped to using undo --index --absolute.

	If the undo extension was turned off and on again, you might loose the			If the undo extension was turned off and on again, you might loose the
	ability to undo to certain repo states. Undoing to repo states before the			ability to undo to certain repo states. Undoing to repo states before the
	missing ones can be forced, but isn't advised unless its known how the			missing ones can be forced, but isn't advised unless its known how the
	before and after states are connected.			before and after states are connected.

	Use keep to maintain working copy changes. With keep, undo mimics hg			Use keep to maintain working copy changes. With keep, undo mimics hg
	unamend and hg uncommit. Specifically, files that exsist currently that			unamend and hg uncommit. Specifically, files that exist currently that
	don't exist at the repo state we are undoing to will remain in your			don't exist at the repo state we are undoing to will remain in your
	working copy but not in your changeset. Maintaining your working copy			working copy but not in your changeset. Maintaining your working copy
	has primarily two downsides: firstly your new working copy won't be clean			has primarily two downsides: firstly your new working copy won't be clean
	so you can't simply redo without cleaning your working copy. Secondly,			so you can't simply redo without cleaning your working copy. Secondly,
	the operation may be slow if your working copy is large. If unsure,			the operation may be slow if your working copy is large. If unsure,
	its generally easier try undo without --keep first and redo if you want			its generally easier try undo without --keep first and redo if you want
	to change this.			to change this.

	Branch limits the scope of an undo to a group of local (draft) changectxs,			Branch limits the scope of an undo to a group of local (draft) changectxs,
	identified by any one member of this group.			identified by any one member of this group.
	"""			"""
	reverseindex = opts.get("index")			reverseindex = opts.get("index")
	relativeundo = not opts.get("absolute")			relativeundo = not opts.get("absolute")
	keep = opts.get("keep")			keep = opts.get("keep")
	branch = opts.get("branch")			branch = opts.get("branch")
				preview = opts.get("preview")

	if branch and reverseindex != 1:			if branch and reverseindex != 1:
	raise error.Abort(_("--branch with --index not supported"))			raise error.Abort(_("--branch with --index not supported"))

				if preview:
				_preview(ui, repo)
				return

	with repo.wlock(), repo.lock(), repo.transaction("undo"):			with repo.wlock(), repo.lock(), repo.transaction("undo"):
	cmdutil.checkunfinished(repo)			cmdutil.checkunfinished(repo)
	cmdutil.bailifchanged(repo)			cmdutil.bailifchanged(repo)
	repo = repo.unfiltered()			repo = repo.unfiltered()
	reverseindex = _computerelative(repo, reverseindex,			reverseindex = _computerelative(repo, reverseindex,
	absolute=not relativeundo,			absolute=not relativeundo,
	branch=branch)			branch=branch)
	if not (opts.get("force") or _gapcheck(repo, reverseindex)):			if not (opts.get("force") or _gapcheck(repo, reverseindex)):
	try:			try:
	inhibit = extensions.find('inhibit')			inhibit = extensions.find('inhibit')
	except KeyError:			except KeyError:
	raise error.Abort(_('undo requires inhibit to work properly'))			raise error.Abort(_('undo requires inhibit to work properly'))
	else:			else:
	ctxts = repo.set(rev)			ctxts = repo.set(rev)
	inhibit.revive(ctxts)			inhibit.revive(ctxts)

				def _preview(ui, repo):
				# Print smartlog like preview of undo
				# Input:
				# ui:
				# repo: mercurial.localrepo
				# Output:
				# None

				reverseindex = 1
				opts = {}
				opts["template"] = "{undopreview}"
				quarkUnsubmitted Not Done I didn't see the definition of `undopreview` template. Maybe provide a fallback in the code. That could be a follow up. quark: I didn't see the definition of `undopreview` template. Maybe provide a fallback in the code.
				reverseindex = _computerelative(repo, reverseindex)
				repo = repo.unfiltered()
				revstring = revsetlang.formatspec("olddraft(%d) + olddraft(0)",
				reverseindex)
				opts['rev'] = [revstring]
				cmdutil.graphlog(ui, repo, None, opts)

	# Tools			# Tools

	def _invertindex(rlog, indexorreverseindex):			def _invertindex(rlog, indexorreverseindex):
	return len(rlog) - 1 - indexorreverseindex			return len(rlog) - 1 - indexorreverseindex

	def _getrevlog(repo, filename):			def _getrevlog(repo, filename):
				quarkUnsubmitted Not Done This is a no-op. You might want `repo = repo.filtered('visible')`. With the change I proposed, it seems we don't need to change the filter here. quark: This is a no-op. You might want `repo = repo.filtered('visible')`. With the change I proposed…
	path = 'undolog/' + filename			path = 'undolog/' + filename
	try:			try:
	return revlog.revlog(repo.vfs, path)			return revlog.revlog(repo.vfs, path)
	except error.RevlogError:			except error.RevlogError:
	# corruption: for now, we can simply nuke all files			# corruption: for now, we can simply nuke all files
	# TODO: log to Scuba			# TODO: log to Scuba
	repo.ui.debug("caught revlog error. %s was probably corrupted\n" % path)			repo.ui.debug("caught revlog error. %s was probably corrupted\n" % path)
	repo.vfs.rmtree('undolog')			repo.vfs.rmtree('undolog')

tests/test-undo.t

	1 files updated, 0 files merged, 0 files removed, 0 files unresolved			1 files updated, 0 files merged, 0 files removed, 0 files unresolved
	$ hg log -r . -T {node}			$ hg log -r . -T {node}
	3ee6a6880888df9e48cdc568b5e835bd3087f8cb (no-eol)			3ee6a6880888df9e48cdc568b5e835bd3087f8cb (no-eol)
	$ cat >> $HGRCPATH <<EOF			$ cat >> $HGRCPATH <<EOF
	> [extensions]			> [extensions]
	> strip =!			> strip =!
	> fbamend =!			> fbamend =!
	> EOF			> EOF

				hg undo --preview test
				$ touch prev1 && hg add prev1 && hg ci -m prev1
				$ cat >> $HGRCPATH <<EOF
				> [templatealias]
				> undopreview = '{if(undonecommits, "Undone")}'
				stashUnsubmitted Not Done Don't need it anymore? stash: Don't need it anymore?
				> EOF
				$ hg undo -p
				@ Undone
				\|
				o
				\|
				\| o
				\|/
				o
				\|
				o
				\|
				o
				\|
				~
				o
				\|
				~
				o
				\|
				~
				stashUnsubmitted Not Done Same here stash: Same here