Patchwork D6683: unshelve: fixes on interactive mode

login
register
mail settings
Submitter phabricator
Date July 24, 2019, 3:39 a.m.
Message ID <differential-rev-PHID-DREV-opts2jhhmuzmhv2phyvh-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/41029/
State Superseded
Headers show

Comments

phabricator - July 24, 2019, 3:39 a.m.
navaneeth.suresh created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This is a follow-up patch to 5162753c4c14 <https://phab.mercurial-scm.org/rHG5162753c4c14c143e6be011b89d6567b5ef50d66> on addressing reviews
  on the commit. This includes fixes on interactive mode.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6683

AFFECTED FILES
  mercurial/shelve.py

CHANGE DETAILS




To: navaneeth.suresh, #hg-reviewers
Cc: mercurial-devel
phabricator - July 24, 2019, 8:52 a.m.
pulkit added a comment.


  This patch does following things:
  
  1. unify logic around creating unshelvectx
  2. changes how date is set in iteractive mode
  3. handles stripping in interavtive mode
  4. compute a matcher only if it's required
  5. and a change around making basename madatory argument
  
  Can we have them in separate patches?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6683/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6683

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 24, 2019, 12:57 p.m.
navaneeth.suresh added a comment.


  In D6683#97731 <https://phab.mercurial-scm.org/D6683#97731>, @pulkit wrote:
  
  > This patch does following things:
  >
  > 1. unify logic around creating unshelvectx
  > 2. changes how date is set in iteractive mode
  > 3. handles stripping in interavtive mode
  > 4. compute a matcher only if it's required
  > 5. and a change around making basename madatory argument
  >
  > Can we have them in separate patches?
  
  Done.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6683/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6683

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 26, 2019, 2:37 p.m.
pulkit added inline comments.

INLINE COMMENTS

> shelve.py:812
> +    Here, we return both the newnode which is created interactively and a
> +    bool to know whether the shelve is partly done or completely done.
>      """

How about:

  Handles creation of unshelve commit and updating the shelve if it was partially unshelved.
  
  If interactive is:
  
    * false: commit all the changes in working directory.
    * true: prompts user to select changes to unshelve and commit them. Update the shelve with remaining changes.
  
  Returns the node of the new commit formed and a bool indicating whether the shelve was partially unshelved.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6683/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6683

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 26, 2019, 3:24 p.m.
navaneeth.suresh added inline comments.

INLINE COMMENTS

> pulkit wrote in shelve.py:812
> How about:
> 
>   Handles creation of unshelve commit and updating the shelve if it was partially unshelved.
>   
>   If interactive is:
>   
>     * false: commit all the changes in working directory.
>     * true: prompts user to select changes to unshelve and commit them. Update the shelve with remaining changes.
>   
>   Returns the node of the new commit formed and a bool indicating whether the shelve was partially unshelved.

Awesome! Amending that right away.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6683/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6683

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel

Patch

diff --git a/mercurial/shelve.py b/mercurial/shelve.py
--- a/mercurial/shelve.py
+++ b/mercurial/shelve.py
@@ -700,7 +700,7 @@ 
             if shfile.exists():
                 shfile.movetobackup()
         cleanupoldbackups(repo)
-def unshelvecontinue(ui, repo, state, opts, basename=None):
+def unshelvecontinue(ui, repo, state, opts, basename):
     """subcommand to continue an in-progress unshelve"""
     # We're finishing off a merge. First parent is our original
     # parent, second is the temporary "fake" commit we're unshelving.
@@ -727,15 +727,8 @@ 
         with repo.ui.configoverride(overrides, 'unshelve'):
             with repo.dirstate.parentchange():
                 repo.setparents(state.parents[0], nodemod.nullid)
-                if not interactive:
-                    ispartialunshelve = False
-                    newnode = repo.commit(text=shelvectx.description(),
-                                        extra=shelvectx.extra(),
-                                        user=shelvectx.user(),
-                                        date=shelvectx.date())
-                else:
-                    newnode, ispartialunshelve = _dounshelveinteractive(ui,
-                        repo, shelvectx, basename, opts)
+                newnode, ispartialunshelve = _createunshelvectx(ui,
+                        repo, shelvectx, basename, interactive, opts)
 
         if newnode is None:
             # If it ended up being a no-op commit, then the normal
@@ -755,10 +748,10 @@ 
         mergefiles(ui, repo, state.wctx, shelvectx)
         restorebranch(ui, repo, state.branchtorestore)
 
+        if not phases.supportinternal(repo):
+            repair.strip(ui, repo, state.nodestoremove, backup=False,
+                         topic='shelve')
         if not ispartialunshelve:
-            if not phases.supportinternal(repo):
-                repair.strip(ui, repo, state.nodestoremove, backup=False,
-                            topic='shelve')
             shelvedstate.clear(repo)
             unshelvecleanup(ui, repo, state.name, opts)
         _restoreactivebookmark(repo, state.activebookmark)
@@ -810,14 +803,25 @@ 
 
     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.
+def _createunshelvectx(ui, repo, shelvectx, basename, interactive, opts):
+    """Creates a commit ctx to unshelve interactively or non-interactively.
+    The user might want to unshelve certain changes only from the stored
+    shelve in interactive. So, we would create two commits. One with requested
+    changes to unshelve at that time and the latter is shelved for future.
+
+    Here, we return both the newnode which is created interactively and a
+    bool to know whether the shelve is partly done or completely done.
     """
     opts['message'] = shelvectx.description()
     opts['interactive-unshelve'] = True
     pats = []
+    if not interactive:
+        newnode = repo.commit(text=shelvectx.description(),
+                              extra=shelvectx.extra(),
+                              user=shelvectx.user(),
+                              date=shelvectx.date())
+        return newnode, False
+
     commitfunc = getcommitfunc(shelvectx.extra(), interactive=True,
                                editor=True)
     newnode = cmdutil.dorecord(ui, repo, commitfunc, None, False,
@@ -826,9 +830,9 @@ 
     snode = repo.commit(text=shelvectx.description(),
                         extra=shelvectx.extra(),
                         user=shelvectx.user(),
-                        date=shelvectx.date())
-    m = scmutil.matchfiles(repo, repo[snode].files())
+                        date=opts.get('date'))
     if snode:
+        m = scmutil.matchfiles(repo, repo[snode].files())
         _shelvecreatedcommit(repo, snode, basename, m)
 
     return newnode, bool(snode)
@@ -868,15 +872,8 @@ 
 
         with repo.dirstate.parentchange():
             repo.setparents(tmpwctx.node(), nodemod.nullid)
-            if not interactive:
-                ispartialunshelve = False
-                newnode = repo.commit(text=shelvectx.description(),
-                                      extra=shelvectx.extra(),
-                                      user=shelvectx.user(),
-                                      date=shelvectx.date())
-            else:
-                newnode, ispartialunshelve = _dounshelveinteractive(ui, repo,
-                                                shelvectx, basename, opts)
+            newnode, ispartialunshelve = _createunshelvectx(ui, repo,
+                                            shelvectx, basename, interactive, opts)
 
         if newnode is None:
             # If it ended up being a no-op commit, then the normal