This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3517

shelve: reduce scope of merge tool config override
ClosedPublic

Authored by martinvonz on May 9 2018, 8:14 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG7932be8b0559: shelve: reduce scope of merge tool config override
Summary

The config override seems to have a much greater scope than it needed
to. I *think* it's only relevant in the while merging files. The
rebase step also cares about the merge tool, but we seem to be
explicitly passing it to rebase (around line 755).

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.May 9 2018, 8:14 PM
yuja added a subscriber: yuja.May 10 2018, 9:15 AM

+ shelvectx = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev,
+ basename, pctx, tmpwctx,
+ shelvectx, branchtorestore,
+ activebookmark)

overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
with ui.configoverride(overrides, 'unshelve'):

Maybe I'm wrong, but doesn't rebase do merge?

martinvonz added a comment.May 10 2018, 11:20 AM
In D3517#55774, @yuja wrote:

+ shelvectx = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev,
+ basename, pctx, tmpwctx,
+ shelvectx, branchtorestore,
+ activebookmark)

overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
with ui.configoverride(overrides, 'unshelve'):

Maybe I'm wrong, but doesn't rebase do merge?

Correct, but we seem to be passing the tool explicitly to the rebase command on line 755. I had checked this before, and should have mentioned it in the commit message. Sorry.

martinvonz edited the summary of this revision. (Show Details)May 10 2018, 11:22 AM
martinvonz retitled this revision from shelve: reduce scope of config override to shelve: reduce scope of merge tool config override.
yuja added a comment.May 11 2018, 8:33 AM
> > +        shelvectx = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev,
> >  +                                          basename, pctx, tmpwctx,
> >  +                                          shelvectx, branchtorestore,
> >  +                                          activebookmark)
> > 
> >   overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
> >   with ui.configoverride(overrides, 'unshelve'):
>
> Maybe I'm wrong, but doesn't rebase do merge?
Correct, but we seem to be passing the tool explicitly to the rebase command on line 755.

Right. Queued this, thanks.

Closed by commit rHG7932be8b0559: shelve: reduce scope of merge tool config override (authored by martinvonz). · Explain WhyMay 11 2018, 3:32 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/shelve.py (36 lines)

Diff 8596

hgext/shelve.py

tmpwctx = pctx tmpwctx = pctx
# The goal is to have a commit structure like so: # The goal is to have a commit structure like so:
# ...-> pctx -> tmpwctx -> shelvectx # ...-> pctx -> tmpwctx -> shelvectx
# where tmpwctx is an optional commit with the user's pending changes # where tmpwctx is an optional commit with the user's pending changes
# and shelvectx is the unshelved changes. Then we merge it all down # and shelvectx is the unshelved changes. Then we merge it all down
# to the original pctx. # to the original pctx.
activebookmark = _backupactivebookmark(repo) activebookmark = _backupactivebookmark(repo)
overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
with ui.configoverride(overrides, 'unshelve'):
tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts, tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,
tmpwctx) tmpwctx)
repo, shelvectx = _unshelverestorecommit(ui, repo, basename) repo, shelvectx = _unshelverestorecommit(ui, repo, 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 = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev,
basename, pctx, tmpwctx, basename, pctx, tmpwctx,
shelvectx, branchtorestore, shelvectx, branchtorestore,
activebookmark) activebookmark)
overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
with ui.configoverride(overrides, 'unshelve'):
mergefiles(ui, repo, pctx, shelvectx) mergefiles(ui, repo, pctx, shelvectx)
restorebranch(ui, repo, branchtorestore) restorebranch(ui, repo, branchtorestore)
_forgetunknownfiles(repo, shelvectx, addedbefore) _forgetunknownfiles(repo, shelvectx, addedbefore)
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)
@command('shelve', @command('shelve',
[('A', 'addremove', None, [('A', 'addremove', None,
_('mark new/missing files as added/removed before shelving')), _('mark new/missing files as added/removed before shelving')),