This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6596

unshelve: add interactive mode
ClosedPublic

Authored by navaneeth.suresh on Jul 2 2019, 8:43 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG5162753c4c14: unshelve: add interactive mode
rHG62953fd58404: unshelve: add interactive mode
Summary

Until now, there is no way to unshelve selected changes only from
the stored shelve as given in issue6162. This patch makes unshelve
perform with certain changes only by adding an interactive mode.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit added a subscriber: pulkit.Jul 2 2019, 8:56 AM

Didn't look at the actual implementation yet.

mercurial/shelve.py
975

The interactive flag should work the same way as it work for revert, commit and shelve commands. That's either ask hunk by hunk or pop up a curses UI.

As you suggested before, maybe we can have -f flag to specify files to unshelve.

tests/test-shelve.t
1204

add a comment here specifying that shelve should not contain c now.

navaneeth.suresh marked an inline comment as done.Jul 2 2019, 9:18 AM
navaneeth.suresh updated this revision to Diff 15739.
navaneeth.suresh added inline comments.Jul 2 2019, 9:19 AM
mercurial/shelve.py
975

will fix this in the next-to-next revision.

pulkit added inline comments.Jul 2 2019, 5:53 PM
mercurial/shelve.py
975

To have this patch in a good state to be reviewed, it will be nice you take out the addition of interactive flag to the patch where you add that functionality.

navaneeth.suresh retitled this revision from unshelve: add interactive mode (issue6162) to unshelve: make unshelve accept files (issue6162).Jul 4 2019, 10:49 AM
navaneeth.suresh edited the summary of this revision. (Show Details)
navaneeth.suresh updated this revision to Diff 15757.
navaneeth.suresh marked 2 inline comments as done.Jul 4 2019, 10:51 AM
navaneeth.suresh added inline comments.
mercurial/shelve.py
975

Updated the patch by replacing the --interactive flag with --files.

navaneeth.suresh marked an inline comment as done.Jul 4 2019, 10:52 AM
pulkit added inline comments.Jul 6 2019, 7:38 PM
mercurial/shelve.py
824–825

We can do filtering of files to unshelve here.

Before this repo.commit we have all the changes, we first commit which are needed to be unshelve, then create a shelve of rest of the changes.

pulkit added a comment.Jul 11 2019, 9:18 AM

It will be nice to add information to commit message about how thing unshelving of a subset works.

mercurial/shelve.py
979–980

Can you explain why we are not processing this if files are present?

navaneeth.suresh marked 2 inline comments as done.Jul 11 2019, 9:24 AM
navaneeth.suresh added inline comments.
mercurial/shelve.py
979–980

okay. on unshelving with --files flag, we don't want to clear the shelvedstate unless all the files in the shelvectx is same as the files which are requested by the user. on unshelving without --files flag, we can clear the shelvedstate right away. that's why i have used this condition.

navaneeth.suresh marked an inline comment as done.Jul 11 2019, 9:25 AM
navaneeth.suresh edited the summary of this revision. (Show Details)Jul 11 2019, 9:33 AM
navaneeth.suresh retitled this revision from unshelve: make unshelve accept files (issue6162) to unshelve: add interactive mode.Jul 11 2019, 2:55 PM
navaneeth.suresh edited the summary of this revision. (Show Details)
navaneeth.suresh updated this revision to Diff 15890.
navaneeth.suresh edited the summary of this revision. (Show Details)Jul 12 2019, 3:46 AM
navaneeth.suresh updated this revision to Diff 15891.
navaneeth.suresh added a commit: rHG62953fd58404: unshelve: add interactive mode.Jul 17 2019, 7:05 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG62953fd58404: unshelve: add interactive mode (authored by navaneeth.suresh). · Explain Why
This revision was automatically updated to reflect the committed changes.
pulkit added inline comments.Jul 18 2019, 12:50 AM
mercurial/commands.py
6162

Can you mark this as EXPERIMENTAL because right now we rebases the whole commit, which leads to conflicts in files which the user might not want to unshelve.

tests/test-shelve.t
1351

We should store information about --interacive in shelve state, which will help us to prevent passing -i here again.

navaneeth.suresh added inline comments.Jul 18 2019, 11:21 AM
mercurial/commands.py
6162

Doing that right away! Will solve this issue later.

tests/test-shelve.t
1351

We only pass -i if we want to do the unshelve interactively again. It's not mandatory to do that. After an interactive shelve, the stored remaining shelve is same as a normal shelve. We can unshelve it without the -i. So, I don't think we have to do that.

pulkit added inline comments.Jul 18 2019, 1:19 PM
tests/test-shelve.t
1351

Looking at the test case:

User wants to interactively unshelve, runs hg unshelve -i. They hit conflicts, resolve conflicts, and then run hg unshelve --continue.

Here, user intended to do an interactive unshelve, and having conflicts should not change what user wanted. Also this is similar to other commands. We preserve the flags which user passed and follow them when the operation is continued.

pulkit added a comment.Jul 23 2019, 9:49 AM

For record: I left more comments about this patch. Mistakenly I did them on the phabricator commit instead of this review. https://phab.mercurial-scm.org/rHG5162753c4c14c143e6be011b89d6567b5ef50d66#51172

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (4 lines)
Mmercurial/shelve.py (61 lines)
Mtests/test-completion.t (2 lines)
Mtests/test-shelve.t (142 lines)

Diff 15892

mercurial/commands.py

return postincoming(ui, repo, modheads, opts.get(r'update'), None, None) return postincoming(ui, repo, modheads, opts.get(r'update'), None, None)
@command('unshelve', @command('unshelve',
[('a', 'abort', None, [('a', 'abort', None,
_('abort an incomplete unshelve operation')), _('abort an incomplete unshelve operation')),
('c', 'continue', None, ('c', 'continue', None,
_('continue an incomplete unshelve operation')), _('continue an incomplete unshelve operation')),
('i', 'interactive', None,
_('use interactive mode')),
pulkitUnsubmitted
Not Done

Can you mark this as EXPERIMENTAL because right now we rebases the whole commit, which leads to conflicts in files which the user might not want to unshelve.

pulkit: Can you mark this as `EXPERIMENTAL` because right now we rebases the whole commit, which leads…
navaneeth.sureshAuthorUnsubmitted
Done

Doing that right away! Will solve this issue later.

navaneeth.suresh: Doing that right away! Will solve this issue later.
('k', 'keep', None, ('k', 'keep', None,
_('keep shelve after unshelving')), _('keep shelve after unshelving')),
('n', 'name', '', ('n', 'name', '',
_('restore shelved change with given name'), _('NAME')), _('restore shelved change with given name'), _('NAME')),
('t', 'tool', '', _('specify merge tool')), ('t', 'tool', '', _('specify merge tool')),
('', 'date', '', ('', 'date', '',
_('set date for temporary commits (DEPRECATED)'), _('DATE'))], _('set date for temporary commits (DEPRECATED)'), _('DATE'))],
_('hg unshelve [[-n] SHELVED]'), _('hg unshelve [OPTION]... [FILE]... [-n SHELVED]'),
helpcategory=command.CATEGORY_WORKING_DIRECTORY) helpcategory=command.CATEGORY_WORKING_DIRECTORY)
def unshelve(ui, repo, *shelved, **opts): def unshelve(ui, repo, *shelved, **opts):
"""restore a shelved change to the working directory """restore a shelved change to the working directory
This command accepts an optional name of a shelved change to This command accepts an optional name of a shelved change to
restore. If none is given, the most recent shelved change is used. restore. If none is given, the most recent shelved change is used.
If a shelved change is applied successfully, the bundle that If a shelved change is applied successfully, the bundle that

mercurial/shelve.py

if node is None: if node is None:
info = {'node': nodemod.hex(shelvectx.node())} info = {'node': nodemod.hex(shelvectx.node())}
shelvedfile(repo, basename, 'shelve').writeinfo(info) shelvedfile(repo, basename, 'shelve').writeinfo(info)
else: else:
shelvectx = repo[node] shelvectx = repo[node]
return repo, shelvectx return repo, shelvectx
def _dounshelveinteractive(ui, repo, shelvectx, basename, opts):
"""The user might want to unshelve certain changes only from the stored
shelve. So, we would create two commits. One with requested changes to
unshelve at that time and the latter is shelved for future.
"""
opts['message'] = shelvectx.description()
pats = []
commitfunc = getcommitfunc(shelvectx.extra(), interactive=True,
editor=True)
newnode = cmdutil.dorecord(ui, repo, commitfunc, None, False,
cmdutil.recordfilter, *pats,
**pycompat.strkwargs(opts))
snode = repo.commit(text=shelvectx.description(),
extra=shelvectx.extra(),
user=shelvectx.user(),
date=shelvectx.date())
m = scmutil.matchfiles(repo, repo[snode].files())
if snode:
_shelvecreatedcommit(repo, snode, basename, m)
return newnode, bool(snode)
def _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, basename, pctx, def _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, basename, pctx,
tmpwctx, shelvectx, branchtorestore, tmpwctx, shelvectx, branchtorestore,
activebookmark): activebookmark):
"""Rebase restored commit from its original location to a destination""" """Rebase restored commit from its original location to a destination"""
# If the shelve is not immediately on top of the commit # If the shelve is not immediately on top of the commit
# we'll be merging with, rebase it to be on top. # we'll be merging with, rebase it to be on top.
if tmpwctx.node() == shelvectx.p1().node(): interactive = opts.get('interactive')
return shelvectx if tmpwctx.node() == shelvectx.p1().node() and not interactive:
# We won't skip on interactive mode because, the user might want to
# unshelve certain changes only.
return shelvectx, False
overrides = { overrides = {
('ui', 'forcemerge'): opts.get('tool', ''), ('ui', 'forcemerge'): opts.get('tool', ''),
('phases', 'new-commit'): phases.secret, ('phases', 'new-commit'): phases.secret,
} }
with repo.ui.configoverride(overrides, 'unshelve'): with repo.ui.configoverride(overrides, 'unshelve'):
ui.status(_('rebasing shelved changes\n')) ui.status(_('rebasing shelved changes\n'))
stats = merge.graft(repo, shelvectx, shelvectx.p1(), stats = merge.graft(repo, shelvectx, shelvectx.p1(),
labels=['shelve', 'working-copy'], labels=['shelve', 'working-copy'],
keepconflictparent=True) keepconflictparent=True)
if stats.unresolvedcount: if stats.unresolvedcount:
tr.close() tr.close()
nodestoremove = [repo.changelog.node(rev) nodestoremove = [repo.changelog.node(rev)
for rev in pycompat.xrange(oldtiprev, len(repo))] for rev in pycompat.xrange(oldtiprev, len(repo))]
shelvedstate.save(repo, basename, pctx, tmpwctx, nodestoremove, shelvedstate.save(repo, basename, pctx, tmpwctx, nodestoremove,
branchtorestore, opts.get('keep'), activebookmark) branchtorestore, opts.get('keep'), activebookmark)
raise error.InterventionRequired( raise error.InterventionRequired(
_("unresolved conflicts (see 'hg resolve', then " _("unresolved conflicts (see 'hg resolve', then "
"'hg unshelve --continue')")) "'hg unshelve --continue')"))
with repo.dirstate.parentchange(): with repo.dirstate.parentchange():
repo.setparents(tmpwctx.node(), nodemod.nullid) repo.setparents(tmpwctx.node(), nodemod.nullid)
if not interactive:
pulkitUnsubmitted
Done

We can do filtering of files to unshelve here.

Before this repo.commit we have all the changes, we first commit which are needed to be unshelve, then create a shelve of rest of the changes.

pulkit: We can do filtering of files to unshelve here. Before this `repo.commit` we have all the…
ispartialunshelve = False
newnode = repo.commit(text=shelvectx.description(), newnode = repo.commit(text=shelvectx.description(),
extra=shelvectx.extra(), extra=shelvectx.extra(),
user=shelvectx.user(), user=shelvectx.user(),
date=shelvectx.date()) date=shelvectx.date())
else:
newnode, ispartialunshelve = _dounshelveinteractive(ui, repo,
shelvectx, basename, opts)
if newnode is None: if newnode is None:
# If it ended up being a no-op commit, then the normal # If it ended up being a no-op commit, then the normal
# merge state clean-up path doesn't happen, so do it # merge state clean-up path doesn't happen, so do it
# here. Fix issue5494 # here. Fix issue5494
merge.mergestate.clean(repo) merge.mergestate.clean(repo)
shelvectx = tmpwctx shelvectx = tmpwctx
msg = _('note: unshelved changes already existed ' msg = _('note: unshelved changes already existed '
'in the working copy\n') 'in the working copy\n')
ui.status(msg) ui.status(msg)
else: else:
shelvectx = repo[newnode] shelvectx = repo[newnode]
hg.updaterepo(repo, tmpwctx.node(), False) hg.updaterepo(repo, tmpwctx.node(), False)
return shelvectx return shelvectx, ispartialunshelve
def _forgetunknownfiles(repo, shelvectx, addedbefore): def _forgetunknownfiles(repo, shelvectx, addedbefore):
# Forget any files that were unknown before the shelve, unknown before # Forget any files that were unknown before the shelve, unknown before
# unshelve started, but are now added. # unshelve started, but are now added.
shelveunknown = shelvectx.extra().get('shelve_unknown') shelveunknown = shelvectx.extra().get('shelve_unknown')
if not shelveunknown: if not shelveunknown:
return return
shelveunknown = frozenset(shelveunknown.split('\0')) shelveunknown = frozenset(shelveunknown.split('\0'))
m = _("shelved change touches missing files") m = _("shelved change touches missing files")
hint = _("run hg status to see which files are missing") hint = _("run hg status to see which files are missing")
raise error.Abort(m, hint=hint) raise error.Abort(m, hint=hint)
def _dounshelve(ui, repo, *shelved, **opts): def _dounshelve(ui, repo, *shelved, **opts):
opts = pycompat.byteskwargs(opts) opts = pycompat.byteskwargs(opts)
abortf = opts.get('abort') abortf = opts.get('abort')
continuef = opts.get('continue') continuef = opts.get('continue')
interactive = opts.get('interactive')
if not abortf and not continuef: if not abortf and not continuef:
cmdutil.checkunfinished(repo) cmdutil.checkunfinished(repo)
shelved = list(shelved) shelved = list(shelved)
if opts.get("name"): if opts.get("name"):
shelved.append(opts["name"]) shelved.append(opts["name"])
if abortf or continuef: if abortf or continuef:
if abortf and continuef: if abortf and continuef:
tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts, tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,
tmpwctx) tmpwctx)
repo, shelvectx = _unshelverestorecommit(ui, repo, tr, basename) repo, shelvectx = _unshelverestorecommit(ui, repo, tr, basename)
_checkunshelveuntrackedproblems(ui, repo, shelvectx) _checkunshelveuntrackedproblems(ui, repo, shelvectx)
branchtorestore = '' branchtorestore = ''
if shelvectx.branch() != shelvectx.p1().branch(): if shelvectx.branch() != shelvectx.p1().branch():
branchtorestore = shelvectx.branch() branchtorestore = shelvectx.branch()
shelvectx = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, shelvectx, ispartialunshelve = _rebaserestoredcommit(ui, repo, opts,
basename, pctx, tmpwctx, tr, oldtiprev, basename, pctx, tmpwctx, shelvectx,
shelvectx, branchtorestore, branchtorestore, activebookmark)
activebookmark)
overrides = {('ui', 'forcemerge'): opts.get('tool', '')} overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
pulkitUnsubmitted
Done

The interactive flag should work the same way as it work for revert, commit and shelve commands. That's either ask hunk by hunk or pop up a curses UI.

As you suggested before, maybe we can have -f flag to specify files to unshelve.

pulkit: The interactive flag should work the same way as it work for revert, commit and shelve commands.
navaneeth.sureshAuthorUnsubmitted
Done

will fix this in the next-to-next revision.

navaneeth.suresh: will fix this in the next-to-next revision.
pulkitUnsubmitted
Done

To have this patch in a good state to be reviewed, it will be nice you take out the addition of interactive flag to the patch where you add that functionality.

pulkit: To have this patch in a good state to be reviewed, it will be nice you take out the addition of…
navaneeth.sureshAuthorUnsubmitted
Done

Updated the patch by replacing the --interactive flag with --files.

navaneeth.suresh: Updated the patch by replacing the `--interactive` flag with `--files`.
with ui.configoverride(overrides, 'unshelve'): with ui.configoverride(overrides, 'unshelve'):
mergefiles(ui, repo, pctx, shelvectx) mergefiles(ui, repo, pctx, shelvectx)
restorebranch(ui, repo, branchtorestore) restorebranch(ui, repo, branchtorestore)
if not ispartialunshelve:
_forgetunknownfiles(repo, shelvectx, addedbefore) _forgetunknownfiles(repo, shelvectx, addedbefore)
pulkitUnsubmitted
Done

Can you explain why we are not processing this if files are present?

pulkit: Can you explain why we are not processing this if files are present?
navaneeth.sureshAuthorUnsubmitted
Done

okay. on unshelving with --files flag, we don't want to clear the shelvedstate unless all the files in the shelvectx is same as the files which are requested by the user. on unshelving without --files flag, we can clear the shelvedstate right away. that's why i have used this condition.

navaneeth.suresh: okay. on unshelving with `--files` flag, we don't want to clear the shelvedstate unless all the…
shelvedstate.clear(repo) shelvedstate.clear(repo)
_finishunshelve(repo, oldtiprev, tr, activebookmark) _finishunshelve(repo, oldtiprev, tr, activebookmark)
unshelvecleanup(ui, repo, basename, opts) unshelvecleanup(ui, repo, basename, opts)
finally: finally:
if tr: if tr:
tr.release() tr.release()
lockmod.release(lock) lockmod.release(lock)

tests/test-completion.t

serve: accesslog, daemon, daemon-postexec, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate, print-url, subrepos serve: accesslog, daemon, daemon-postexec, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate, print-url, subrepos
shelve: addremove, unknown, cleanup, date, delete, edit, keep, list, message, name, patch, interactive, stat, include, exclude shelve: addremove, unknown, cleanup, date, delete, edit, keep, list, message, name, patch, interactive, stat, include, exclude
status: all, modified, added, removed, deleted, clean, unknown, ignored, no-status, terse, copies, print0, rev, change, include, exclude, subrepos, template status: all, modified, added, removed, deleted, clean, unknown, ignored, no-status, terse, copies, print0, rev, change, include, exclude, subrepos, template
summary: remote summary: remote
tag: force, local, rev, remove, edit, message, date, user tag: force, local, rev, remove, edit, message, date, user
tags: template tags: template
tip: patch, git, style, template tip: patch, git, style, template
unbundle: update unbundle: update
unshelve: abort, continue, keep, name, tool, date unshelve: abort, continue, interactive, keep, name, tool, date
update: clean, check, merge, date, rev, tool update: clean, check, merge, date, rev, tool
verify: full verify: full
version: template version: template
$ hg init a $ hg init a
$ cd a $ cd a
$ echo fee > fee $ echo fee > fee
$ hg ci -q -Amfee $ hg ci -q -Amfee

tests/test-shelve.t

-- trying to pull in the shelve bits -- trying to pull in the shelve bits
-- unshelve should abort otherwise, it'll eat my second parent. -- unshelve should abort otherwise, it'll eat my second parent.
$ hg unshelve $ hg unshelve
abort: outstanding uncommitted merge abort: outstanding uncommitted merge
(use 'hg commit' or 'hg merge --abort') (use 'hg commit' or 'hg merge --abort')
[255] [255]
$ cd .. $ cd ..
-- test for interactive mode on unshelve
$ hg init a
$ cd a
$ echo > b
$ hg ci -Am b
adding b
$ echo > c
$ echo > d
$ hg add .
adding c
adding d
$ hg shelve
shelved as default
0 files updated, 0 files merged, 2 files removed, 0 files unresolved
$ echo > e
$ hg add e
$ hg ci -m e
$ hg shelve --patch
default (1s ago) changes to: b
diff --git a/c b/c
new file mode 100644
--- /dev/null
+++ b/c
@@ -0,0 +1,1 @@
+
diff --git a/d b/d
new file mode 100644
--- /dev/null
+++ b/d
@@ -0,0 +1,1 @@
+
$ hg unshelve -i <<EOF
> y
> y
> y
> n
> EOF
unshelving change 'default'
rebasing shelved changes
diff --git a/c b/c
new file mode 100644
examine changes to 'c'? [Ynesfdaq?] y
@@ -0,0 +1,1 @@
pulkitUnsubmitted
Done

add a comment here specifying that shelve should not contain c now.

pulkit: add a comment here specifying that shelve should not contain `c` now.
+
record change 1/2 to 'c'? [Ynesfdaq?] y
diff --git a/d b/d
new file mode 100644
examine changes to 'd'? [Ynesfdaq?] y
@@ -0,0 +1,1 @@
+
record change 2/2 to 'd'? [Ynesfdaq?] n
$ ls
b
c
e
-- shelve should not contain `c` now
$ hg shelve --patch
default (1s ago) changes to: b
diff --git a/d b/d
new file mode 100644
--- /dev/null
+++ b/d
@@ -0,0 +1,1 @@
+
$ hg unshelve -i <<EOF
> y
> y
> EOF
unshelving change 'default'
rebasing shelved changes
diff --git a/d b/d
new file mode 100644
examine changes to 'd'? [Ynesfdaq?] y
@@ -0,0 +1,1 @@
+
record this change to 'd'? [Ynesfdaq?] y
$ ls
b
c
d
e
$ hg shelve --list
-- now, unshelve selected changes from a file
$ echo B > foo
$ hg add foo
$ hg ci -m 'add B to foo'
$ cat > foo <<EOF
> A
> B
> C
> EOF
$ hg shelve
shelved as default
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cat foo
B
$ hg unshelve -i <<EOF
> y
> y
> n
> EOF
unshelving change 'default'
rebasing shelved changes
diff --git a/foo b/foo
2 hunks, 2 lines changed
examine changes to 'foo'? [Ynesfdaq?] y
@@ -1,1 +1,2 @@
+A
B
record change 1/2 to 'foo'? [Ynesfdaq?] y
@@ -1,1 +2,2 @@
B
+C
record change 2/2 to 'foo'? [Ynesfdaq?] n
$ cat foo
A
B
$ hg shelve --patch
default (1s ago) changes to: add B to foo
diff --git a/foo b/foo
--- a/foo
+++ b/foo
@@ -1,2 +1,3 @@
A
B
+C
pulkitUnsubmitted
Not Done

We should store information about --interacive in shelve state, which will help us to prevent passing -i here again.

pulkit: We should store information about `--interacive` in shelve state, which will help us to prevent…
navaneeth.sureshAuthorUnsubmitted
Done

We only pass -i if we want to do the unshelve interactively again. It's not mandatory to do that. After an interactive shelve, the stored remaining shelve is same as a normal shelve. We can unshelve it without the -i. So, I don't think we have to do that.

navaneeth.suresh: We only pass `-i` if we want to do the unshelve interactively again. It's not mandatory to do…
pulkitUnsubmitted
Not Done

Looking at the test case:

User wants to interactively unshelve, runs hg unshelve -i. They hit conflicts, resolve conflicts, and then run hg unshelve --continue.

Here, user intended to do an interactive unshelve, and having conflicts should not change what user wanted. Also this is similar to other commands. We preserve the flags which user passed and follow them when the operation is continued.

pulkit: Looking at the test case: User wants to interactively unshelve, runs `hg unshelve -i`. They…