| Submitter | phabricator |
|---|---|
| Date | June 23, 2018, 12:48 p.m. |
| Message ID | <b884462784fb8946e1a241ef06c62246@localhost.localdomain> |
| Download | mbox | patch |
| Permalink | /patch/32392/ |
| State | Not Applicable |
| Headers | show |
Comments
> - def _prepareabortorcontinue(self, isabort): > + def _prepareabortorcontinue(self, isabort, opts={}): Mutable default value should be avoided. It can be `backup=True` since we aren't processing command-level thingy here. And please make sure tests pass before sending out patches. > @@ -828,7 +829,8 @@ > else: > ui.status(_('there will be no conflict, you can rebase\n')) > finally: > - _origrebase(ui, repo, abort=True) > + opts = {'abort':True, 'no_backup':True} > + _origrebase(ui, repo, **opts) Maybe we can refactor the dryrun handling in a way we don't have to pass around the no_backup flag. IIUC, `_origrebase()` is a high-level function which has many parameter/state checks, but what we want is to cancel the in-memory session we've made just before. No user error should occur. > @@ -1588,7 +1590,10 @@ > > # Strip from the first rebased revision > if rebased: > - repair.strip(repo.ui, repo, strippoints) > + if nobackup: > + repair.strip(repo.ui, repo, strippoints, backup=False) > + else: > + repair.strip(repo.ui, repo, strippoints) This can be written as `backup=not nobackup` or `backup=backup`.
yuja added a comment. > - def _prepareabortorcontinue(self, isabort): + def _prepareabortorcontinue(self, isabort, opts={}): Mutable default value should be avoided. It can be `backup=True` since we aren't processing command-level thingy here. And please make sure tests pass before sending out patches. > @@ -828,7 +829,8 @@ > > else: > ui.status(_('there will be no conflict, you can rebase\n')) > finally: > > - _origrebase(ui, repo, abort=True) + opts = {'abort':True, 'no_backup':True} + _origrebase(ui, repo, **opts) Maybe we can refactor the dryrun handling in a way we don't have to pass around the no_backup flag. IIUC, `_origrebase()` is a high-level function which has many parameter/state checks, but what we want is to cancel the in-memory session we've made just before. No user error should occur. > @@ -1588,7 +1590,10 @@ > > 1. Strip from the first rebased revision if rebased: > - repair.strip(repo.ui, repo, strippoints) + if nobackup: + repair.strip(repo.ui, repo, strippoints, backup=False) + else: + repair.strip(repo.ui, repo, strippoints) This can be written as `backup=not nobackup` or `backup=backup`. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3827 To: khanchi97, #hg-reviewers Cc: yuja, mercurial-devel
Patch
diff --git a/tests/test-rebase-inmemory.t b/tests/test-rebase-inmemory.t --- a/tests/test-rebase-inmemory.t +++ b/tests/test-rebase-inmemory.t @@ -212,7 +212,6 @@ rebasing 3:055a42cdd887 "d" rebasing 4:e860deea161a "e" there will be no conflict, you can rebase - saved backup bundle to $TESTTMP/repo1/repo2/skrepo/.hg/strip-backup/c83b1da5b1ae-f1e0beb9-backup.hg rebase aborted $ hg diff diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -325,7 +325,7 @@ skippedset.update(obsoleteextinctsuccessors) _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset) - def _prepareabortorcontinue(self, isabort): + def _prepareabortorcontinue(self, isabort, opts={}): try: self.restorestatus() self.collapsemsg = restorecollapsemsg(self.repo, isabort) @@ -341,8 +341,9 @@ hint = _('use "hg rebase --abort" to clear broken state') raise error.Abort(msg, hint=hint) if isabort: - return abort(self.repo, self.originalwd, self.destmap, - self.state, activebookmark=self.activebookmark) + nobackup = opts.get(r'no_backup') + return abort(self.repo, self.originalwd, self.destmap, self.state, + activebookmark=self.activebookmark, nobackup=nobackup) def _preparenewrebase(self, destmap): if not destmap: @@ -828,7 +829,8 @@ else: ui.status(_('there will be no conflict, you can rebase\n')) finally: - _origrebase(ui, repo, abort=True) + opts = {'abort':True, 'no_backup':True} + _origrebase(ui, repo, **opts) elif inmemory: try: # in-memory merge doesn't support conflicts, so if we hit any, abort @@ -889,7 +891,7 @@ ms = mergemod.mergestate.read(repo) mergeutil.checkunresolved(ms) - retcode = rbsrt._prepareabortorcontinue(abortf) + retcode = rbsrt._prepareabortorcontinue(abortf, opts=opts) if retcode is not None: return retcode else: @@ -1543,7 +1545,7 @@ return False -def abort(repo, originalwd, destmap, state, activebookmark=None): +def abort(repo, originalwd, destmap, state, activebookmark=None, nobackup=False): '''Restore the repository to its original state. Additional args: activebookmark: the name of the bookmark that should be active after the @@ -1588,7 +1590,10 @@ # Strip from the first rebased revision if rebased: - repair.strip(repo.ui, repo, strippoints) + if nobackup: + repair.strip(repo.ui, repo, strippoints, backup=False) + else: + repair.strip(repo.ui, repo, strippoints) if activebookmark and activebookmark in repo._bookmarks: bookmarks.activate(repo, activebookmark)