The good news is that you don't need to implement all that. There is an evolve extension which has already implemented that: https://www.mercurial-scm.org/repo/evolve/file/8494015ec24b/hgext3rd/evolve/cmdrewrite.py#l460. You can import that from there.

If you plan to import that from there, I suggest notifying @lothiraldan about that too so that they can manage things in evolve.

Thanks for the review @pulkit . By importing do you suggest importing the code for interactive or someway is there is someway to link evolve to hg-stable ?

In D5792#84959, @taapas1128 wrote:

Thanks for the review @pulkit . By importing do you suggest importing the code for interactive or someway is there is someway to link evolve to hg-stable ?

I mean importing the code.

taapas1128 updated this revision to Diff 13701.Feb 2 2019, 1:11 PM

@pulkit I have made an attempt to import the code please review. @lothiraldan can you please review and help me if I have missed out any portion of interactive from evolve

taapas1128 edited the summary of this revision. (Show Details)Feb 2 2019, 1:16 PM

In D5792#85088, @taapas1128 wrote:

@pulkit I have made an attempt to import the code please review. @lothiraldan can you please review and help me if I have missed out any portion of interactive from evolve

@taapas1128 haven't looked at the code in detail yet, but there are tests in evolve which should also be moved.

taapas1128 edited the summary of this revision. (Show Details)Feb 4 2019, 9:12 AM

taapas1128 updated this revision to Diff 13737.

taapas1128 updated this revision to Diff 13738.Feb 4 2019, 9:19 AM

@pulkit I was able to import the tests and make them compatible with hg-stable . Some tests which include commands like hg obslog , hg amend --extract , and hg uncommit -n which are not part of hg-stable but evolve are not running else all tests are now running smoothly .

In D5792#85236, @taapas1128 wrote:

@pulkit I was able to import the tests and make them compatible with hg-stable . Some tests which include commands like hg obslog , hg amend --extract , and hg uncommit -n which are not part of hg-stable but evolve are not running else all tests are now running smoothly .

hg amend --extract is just another name for hg uncommit.
You can move --note flag to core uncommit as a part of different patch.
Related to obslog, you can do hg log -r . -r precursor(.) or hg diff -r . -r precursor(.) but I don't think that will be sufficient.

@pulkit I have added some inline comments . So what do you suggest one way can be merging this and I will send a follow up for integrating -n to hg uncommitand then amending the tests. The other way is I should send a request for -n flag and then make all these changes again as a follow up pr for that request.

tests/test-uncommit-interactive.t
135 ↗	(On Diff #13738)	`hg amend --extract` uses `-n` flag here so it cant be completed with `hg uncommit` alone `-n` integration will be necessary. Also `hg obslog` is the same test so without this it can also not be replaced.
208 ↗	(On Diff #13738)	Same problem here.

In D5792#86123, @taapas1128 wrote:

@pulkit I have added some inline comments . So what do you suggest one way can be merging this and I will send a follow up for integrating -n to hg uncommitand then amending the tests. The other way is I should send a request for -n flag and then make all these changes again as a follow up pr for that request.

It will be nice if you send a patch for -n flag first, then rebase these patch on top of that.

taapas1128 updated this revision to Diff 13915.Feb 8 2019, 9:29 AM

@pulkit Apparently it seems removing -n flag altogether will not interfere with the tests and they can be completed without that flag too . I have added inline comments please review. Also I am unable to use hg diff -r . -r precursor(.) so I used hg log -G --hidden in that place.

tests/test-uncommit-interactive.t
134 ↗	(On Diff #13915)	replaced hg amend --extract by hg uncommit -i
253 ↗	(On Diff #13915)	Removed `-n` flag here

@pulkit Can you please review this . I did not send a patch regarding -n flag because tests could be completed without it and also it would mean i would have to import large chunks of code.

The import looks fine to me. I will let someone else review the code because I authored this code in first place in evolve extension. Also it will be nice if you specify that the code is imported from evolve extension and also specify the commit hash from which you imported.

@pulkit I have updated the description .

In D5792#87115, @taapas1128 wrote:

@pulkit I have updated the description .

Great thanks! As mentioned before I will let someone else review this :)

I'd fix this one style nit in flight (this actually looks good), but it no longer applies - could you rebase it for me?

hgext/uncommit.py
215	Need an extra blank line here.

This revision now requires changes to proceed.Feb 15 2019, 10:56 PM

taapas1128 edited the summary of this revision. (Show Details)Feb 16 2019, 2:31 AM

taapas1128 updated this revision to Diff 14132.

@durin42 I have made the change. Please review.

Patch still fails to apply. Would you please rebase it as requested?

(I get patch application problems in uncommit.py, and the parent of the diff isn't in my repo so I can't do the rebase easily locally.)

@durin42 to which rev should I rebase it to ?

Anything recent. @ on https://mercurial-scm.org/hg is fine.

@durin42 I have sent the patch for hg-stable repo (https://www.mercurial-scm.org/repo/hg-stable) . Maybe it is this the reason you are unable to apply the patch. Should I send a patch for hg repo ?

In D5792#87320, @taapas1128 wrote:

@durin42 I have sent the patch for hg-stable repo (https://www.mercurial-scm.org/repo/hg-stable) . Maybe it is this the reason you are unable to apply the patch. Should I send a patch for hg repo ?

Yes, you should rebase your patch on default branch of hg repo. You don't need to clone again, you can pull from https://mercurial-scm.org/hg and then rebase.

taapas1128 updated this revision to Diff 14139.Feb 18 2019, 7:29 AM

Thank you for your help @pulkit . I have rebased it to the top and updated the diff.

@durin42 please review this whenever you are free.

Closed by commit rHGedd5c8dc812e: uncommit: added interactive mode(issue6062) (authored by taapas1128). · Explain WhyFeb 20 2019, 3:16 PM

This revision was automatically updated to reflect the committed changes.

I'll drop this patch from the queue because I think there's a lot that can be cleaned up. I tried to re-unify _fixdirstate() and _uncommitdirstate() myself. This is my first step:

diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -138,7 +138,7 @@ def _fixdirstate(repo, oldctx, newctx, m
         ds.copy(src, dst)
 
 
-def _uncommitdirstate(repo, oldctx, match, interactive):
+def _uncommitdirstate(repo, oldctx, newctx, match, interactive):
     """Fix the dirstate after switching the working directory from
     oldctx to a copy of oldctx not containing changed files matched by
     match.
@@ -217,21 +217,13 @@ def _uncommitdirstate(repo, oldctx, matc
                 ds.remove(f)
 
     # Merge old parent and old working dir copies
-    oldcopies = {}
-    if interactive:
-        # Interactive had different meaning of the variables so restoring the
-        # original meaning to use them
-        m, a, r = repo.status(oldctx.p1(), oldctx, match=match)[:3]
-    for f in (m + a):
-        src = oldctx[f].renamed()
-        if src:
-            oldcopies[f] = src[0]
+    oldcopies = copiesmod.pathcopies(newctx, oldctx, match)
     oldcopies.update(copies)
     copies = dict((dst, oldcopies.get(src, src))
                   for dst, src in oldcopies.iteritems())
     # Adjust the dirstate copies
     for dst, src in copies.iteritems():
-        if (src not in ctx or dst in ctx or ds[dst] != 'a'):
+        if (src not in newctx or dst in newctx or ds[dst] != 'a'):
             src = None
         ds.copy(src, dst)
 
@@ -285,11 +277,11 @@ def uncommit(ui, repo, *pats, **opts):
                 # Fully removed the old commit
                 mapping[old.node()] = ()
 
-            scmutil.cleanupnodes(repo, mapping, 'uncommit', fixphase=True)
-
             with repo.dirstate.parentchange():
                 repo.dirstate.setparents(newid, node.nullid)
-                _uncommitdirstate(repo, old, match, interactive)
+                _uncommitdirstate(repo, old, repo[newid], match, interactive)
+
+            scmutil.cleanupnodes(repo, mapping, 'uncommit', fixphase=True)
 
 def _interactiveuncommit(ui, repo, old, match):
     """ The function which contains all the logic for interactively uncommiting

Is there a reason that's broken? Tests seem to pass (test-uncommit.t passes). If not, could you try to continue in the direction of unifying the two method again?

hgext/uncommit.py
139	Much of this duplicates `_fixdirstate()`. We must be able to share some of this code.
173	Unnecessary (done on line 269)
184	Why did this moved here? I just moved it down in D5660. I really should have written a better commit message explaining what the problem was. But it's also unclear why it should be moved back.

@martinvonz Sure I will try to unify _fixdirstate() and _uncommitdirstate(). I will follow on the advancements you made. Should I send the new patch as a follow up or and altogether new patch making the amends in this?

hgext/uncommit.py
173	Will look into it.
184	Oh that was a mistake made due to rebasing on top . I will shift that back.

In D5792#87534, @taapas1128 wrote:

@martinvonz Sure I will try to unify _fixdirstate() and _uncommitdirstate(). I will follow on the advancements you made. Should I send the new patch as a follow up or and altogether new patch making the amends in this?

A new patch please (I've dropped this version). Just pull from the hg-commited repo and make sure it applies there. Note that you'll probably get an obsmarker when you pull, so you may need to hg touch some version of this commit in your repo to revive it.

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	13668		Feb 1 2019, 2:38 AM	★	★
Diff 2	13669		Feb 1 2019, 2:42 AM	★	★
Diff 3	13701		Feb 2 2019, 1:11 PM	★	★
Diff 4	13737		Feb 4 2019, 9:12 AM	★	★
Diff 5	13738		Feb 4 2019, 9:19 AM	★	★
Diff 6	13915		Feb 8 2019, 9:29 AM	★	★
Diff 7	14132		Feb 16 2019, 2:31 AM	★	★
Diff 8	14139		Feb 18 2019, 7:29 AM	★	★
Diff 9	14144	rHGedd5c8dc812e36784a79e94370291a7885c5f4bd	Feb 1 2019, 2:03 AM	★	★

Commit	Parents	Author	Summary	Date
		Taapas Agrawal		Feb 1 2019, 2:03 AM

Diff 13668

hgext/uncommit.py

	for dst, src in oldcopies.iteritems())			for dst, src in oldcopies.iteritems())
	# Adjust the dirstate copies			# Adjust the dirstate copies
	for dst, src in copies.iteritems():			for dst, src in copies.iteritems():
	if (src not in newctx or dst in newctx or ds[dst] != 'a'):			if (src not in newctx or dst in newctx or ds[dst] != 'a'):
	src = None			src = None
	ds.copy(src, dst)			ds.copy(src, dst)

	@command('uncommit',			@command('uncommit',
	[('', 'keep', False, _('allow an empty commit after uncommiting')),			[('i', 'interactive', None, _('use interactive mode')),
				martinvonzUnsubmitted Not Done Much of this duplicates `_fixdirstate()`. We must be able to share some of this code. martinvonz: Much of this duplicates `_fixdirstate()`. We must be able to share some of this code.
				('', 'keep', False, _('allow an empty commit after uncommiting')),
	] + commands.walkopts,			] + commands.walkopts,
	_('[OPTION]... [FILE]...'),			_('[OPTION]... [FILE]...'),
	helpcategory=command.CATEGORY_CHANGE_MANAGEMENT)			helpcategory=command.CATEGORY_CHANGE_MANAGEMENT)
	def uncommit(ui, repo, pats, *opts):			def uncommit(ui, repo, pats, *opts):
	"""uncommit part or all of a local changeset			"""uncommit part or all of a local changeset

	This command undoes the effect of a local commit, returning the affected			This command undoes the effect of a local commit, returning the affected
	files to their uncommitted state. This means that files modified or			files to their uncommitted state. This means that files modified or
	deleted in the changeset will be left unchanged, and so will remain			deleted in the changeset will be left unchanged, and so will remain
	modified in the working directory.			modified in the working directory.

	If no files are specified, the commit will be pruned, unless --keep is			If no files are specified, the commit will be pruned, unless --keep is
	given.			given.
	"""			"""

	opts = pycompat.byteskwargs(opts)			opts = pycompat.byteskwargs(opts)

	with repo.wlock(), repo.lock():			with repo.wlock(), repo.lock():

	if not pats and not repo.ui.configbool('experimental',			if not pats and not repo.ui.configbool('experimental',
	'uncommitondirtywdir'):			'uncommitondirtywdir'):
	cmdutil.bailifchanged(repo)			cmdutil.bailifchanged(repo)
	old = repo['.']			old = repo['.']
	rewriteutil.precheck(repo, [old.rev()], 'uncommit')			rewriteutil.precheck(repo, [old.rev()], 'uncommit')
	if len(old.parents()) > 1:			if len(old.parents()) > 1:
	raise error.Abort(_("cannot uncommit merge changeset"))			raise error.Abort(_("cannot uncommit merge changeset"))

	with repo.transaction('uncommit'):			with repo.transaction('uncommit'):
	match = scmutil.match(old, pats, opts)			match = scmutil.match(old, pats, opts)
	keepcommit = opts.get('keep') or pats			keepcommit = opts.get('keep') or pats
	newid = _commitfiltered(repo, old, match, keepcommit)			newid = _commitfiltered(repo, old, match, keepcommit)
	if newid is None:			if newid is None:
	ui.status(_("nothing to uncommit\n"))			ui.status(_("nothing to uncommit\n"))
				martinvonzUnsubmitted Not Done Unnecessary (done on line 269) martinvonz: Unnecessary (done on line 269)
				taapas1128AuthorUnsubmitted Not Done Will look into it. taapas1128: Will look into it.
	return 1			return 1

	mapping = {}			mapping = {}
	if newid != old.p1().node():			if newid != old.p1().node():
	# Move local changes on filtered changeset			# Move local changes on filtered changeset
	mapping[old.node()] = (newid,)			mapping[old.node()] = (newid,)
	else:			else:
	# Fully removed the old commit			# Fully removed the old commit
	mapping[old.node()] = ()			mapping[old.node()] = ()

	scmutil.cleanupnodes(repo, mapping, 'uncommit', fixphase=True)			scmutil.cleanupnodes(repo, mapping, 'uncommit', fixphase=True)
				martinvonzUnsubmitted Not Done Why did this moved here? I just moved it down in D5660. I really should have written a better commit message explaining what the problem was. But it's also unclear why it should be moved back. martinvonz: Why did this moved here? I just moved it down in D5660. I really should have written a better…
				taapas1128AuthorUnsubmitted Not Done Oh that was a mistake made due to rebasing on top . I will shift that back. taapas1128: Oh that was a mistake made due to rebasing on top . I will shift that back.

	with repo.dirstate.parentchange():			with repo.dirstate.parentchange():
	repo.dirstate.setparents(newid, node.nullid)			repo.dirstate.setparents(newid, node.nullid)
	s = old.p1().status(old, match=match)			s = old.p1().status(old, match=match)
	_fixdirstate(repo, old, repo[newid], s)			_fixdirstate(repo, old, repo[newid], s)

				if opts.get(r'interactive'):
				opts.pop(r'interactive')
				ret = cmdutil.dorecord(ui, repo, commands.commit, None, False,
				cmdutil.recordfilter, *pats,
				**opts)
	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, **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`)
	"""			"""

	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
				durin42Unsubmitted Done Need an extra blank line here. durin42: Need an extra blank line here.
	curctx = repo['.']			curctx = repo['.']

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

tests/test-uncommit.t

	Test uncommit - set up the config			Test uncommit - set up the config

	$ cat >> $HGRCPATH <<EOF			$ cat >> $HGRCPATH <<EOF
				> [ui]
				> interactive = true
	> [experimental]			> [experimental]
	> evolution.createmarkers=True			> evolution.createmarkers=True
	> evolution.allowunstable=True			> evolution.allowunstable=True
	> [extensions]			> [extensions]
	> uncommit =			> uncommit =
	> drawdag=$TESTDIR/drawdag.py			> drawdag=$TESTDIR/drawdag.py
	> EOF			> EOF


	If no files are specified, the commit will be pruned, unless --keep is			If no files are specified, the commit will be pruned, unless --keep is
	given.			given.

	(use 'hg help -e uncommit' to show help for the uncommit extension)			(use 'hg help -e uncommit' to show help for the uncommit extension)

	options ([+] can be repeated):			options ([+] can be repeated):

				-i --interactive use interactive mode
	--keep allow an empty commit after uncommiting			--keep allow an empty commit after uncommiting
	-I --include PATTERN [+] include names matching the given patterns			-I --include PATTERN [+] include names matching the given patterns
	-X --exclude PATTERN [+] exclude names matching the given patterns			-X --exclude PATTERN [+] exclude names matching the given patterns

	(some details hidden, use --verbose to show complete help)			(some details hidden, use --verbose to show complete help)

	Uncommit with no commits should fail			Uncommit with no commits should fail

	\| o 3:49bb009ca26078726b8870f1edb29fae8f7618f5 b 1			\| o 3:49bb009ca26078726b8870f1edb29fae8f7618f5 b 1
	\| \|			\| \|
	o \| 2:990982b7384266e691f1bc08ca36177adcd1c8a9 a 3			o \| 2:990982b7384266e691f1bc08ca36177adcd1c8a9 a 3
	\| \|			\| \|
	o \| 1:24d38e3cf160c7b6f5ffe82179332229886a6d34 a 2			o \| 1:24d38e3cf160c7b6f5ffe82179332229886a6d34 a 2
	\|/			\|/
	o 0:ea4e33293d4d274a2ba73150733c2612231f398c a 1			o 0:ea4e33293d4d274a2ba73150733c2612231f398c a 1

				Test for interactive mode
				$ hg init repo3
				$ touch x
				$ hg add x
				$ hg commit -m "added x"
				$ hg uncommit -i<<EOF
				> y
				> EOF
				diff --git a/x b/x
				new file mode 100644
				examine changes to 'x'? [Ynesfdaq?] y

				abort: empty commit message
				[255]

			Path	Packages
M			hgext/uncommit.py (9 lines)
M			tests/test-uncommit.t (17 lines)

This is an archive of the discontinued Mercurial Phabricator instance.

uncommit: added interactive mode(issue6062)
ClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 13668

hgext/uncommit.py

tests/test-uncommit.t

This is an archive of the discontinued Mercurial Phabricator instance.

uncommit: added interactive mode(issue6062)ClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 13668

hgext/uncommit.py

tests/test-uncommit.t

uncommit: added interactive mode(issue6062)
ClosedPublic

Revision Contents
Changeset List