This is an archive of the discontinued Mercurial Phabricator instance.

unshelve: add interactive mode
ClosedPublic

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

Details

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
990

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.
mercurial/shelve.py
990

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

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

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
990

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
842

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.

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

mercurial/shelve.py
1003–1004

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
1003–1004

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.
This revision was not accepted when it landed; it landed in state Needs Review.
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.

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.

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