[PATCH 3 of 4 V2] ui: use the new configoverride context manager instead of saving/restoring
Gregory Szorc
gregory.szorc at gmail.com
Wed May 13 20:29:35 CDT 2015
> On May 13, 2015, at 17:42, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>
>
>
>> On 05/13/2015 05:26 PM, Laurent Charignon wrote:
>> # HG changeset patch
>> # User Laurent Charignon <lcharignon at fb.com>
>> # Date 1431562081 25200
>> # Wed May 13 17:08:01 2015 -0700
>> # Node ID cb2f0824776ea5d964813ab4bc5213e03b4e45fc
>> # Parent 480dbca00f19fb90b59f9c6fd511e4806fb642c4
>> ui: use the new configoverride context manager instead of saving/restoring
>>
>> This patch changes all the point in the code (except two) where we used the
>> saving restoring logic for config to use the new configoverride contextmanager.
>> This makes the code more legible.
>>
>> diff --git a/hgext/histedit.py b/hgext/histedit.py
>> --- a/hgext/histedit.py
>> +++ b/hgext/histedit.py
>> @@ -386,16 +386,12 @@
>> """
>> phasemin = src.phase()
>> def commitfunc(**kwargs):
>> - phasebackup = repo.ui.backupconfig('phases', 'new-commit')
>> - try:
>> - repo.ui.setconfig('phases', 'new-commit', phasemin,
>> - 'histedit')
>> + with repo.ui.configoverride('phases', 'new-commit', phasemin,
>> + 'histedit'):
>> extra = kwargs.get('extra', {}).copy()
>> extra['histedit_source'] = src.hex()
>> kwargs['extra'] = extra
>> return repo.commit(**kwargs)
>> - finally:
>> - repo.ui.restoreconfig(phasebackup)
>> return commitfunc
>>
>> def applychanges(ui, repo, ctx, opts):
>> @@ -574,14 +570,11 @@
>> # here. This is sufficient to solve issue3681 anyway.
>> extra['histedit_source'] = '%s,%s' % (ctx.hex(), oldctx.hex())
>> commitopts['extra'] = extra
>> - phasebackup = repo.ui.backupconfig('phases', 'new-commit')
>> - try:
>> - phasemin = max(ctx.phase(), oldctx.phase())
>> - repo.ui.setconfig('phases', 'new-commit', phasemin, 'histedit')
>> + phasemin = max(ctx.phase(), oldctx.phase())
>> + with repo.ui.configoverride('phases', 'new-commit', phasemin,
>> + 'histedit'):
>> n = collapse(repo, ctx, repo[newnode], commitopts,
>> skipprompt=self.skipprompt())
>> - finally:
>> - repo.ui.restoreconfig(phasebackup)
>> if n is None:
>> return ctx, []
>> hg.update(repo, n)
>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>> --- a/hgext/rebase.py
>> +++ b/hgext/rebase.py
>> @@ -540,15 +540,12 @@
>> if extrafn:
>> extrafn(ctx, extra)
>>
>> - backup = repo.ui.backupconfig('phases', 'new-commit')
>> - try:
>> - targetphase = max(ctx.phase(), phases.draft)
>> - repo.ui.setconfig('phases', 'new-commit', targetphase, 'rebase')
>> + targetphase = max(ctx.phase(), phases.draft)
>> + with repo.ui.configoverride('phases', 'new-commit', targetphase,
>> + 'rebase'):
>> # Commit might fail if unresolved files exist
>> newnode = repo.commit(text=commitmsg, user=ctx.user(),
>> date=ctx.date(), extra=extra, editor=editor)
>> - finally:
>> - repo.ui.restoreconfig(backup)
>>
>> repo.dirstate.setbranch(repo[newnode].branch())
>> dsguard.close()
>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>> --- a/hgext/shelve.py
>> +++ b/hgext/shelve.py
>> @@ -177,16 +177,15 @@
>> hasmq = util.safehasattr(repo, 'mq')
>> if hasmq:
>> saved, repo.mq.checkapplied = repo.mq.checkapplied, False
>> - backup = repo.ui.backupconfig('phases', 'new-commit')
>> - try:
>> - repo.ui. setconfig('phases', 'new-commit', phases.secret)
>> - editor = cmdutil.getcommiteditor(editform='shelve.shelve', **opts)
>> - return repo.commit(message, user, opts.get('date'), match,
>> - editor=editor)
>> - finally:
>> - repo.ui.restoreconfig(backup)
>> - if hasmq:
>> - repo.mq.checkapplied = saved
>> + with repo.ui.configoverride('phases', 'new-commit', phases.secret):
>> + try:
>> + editor = cmdutil.getcommiteditor(editform='shelve.shelve',
>> + **opts)
>> + return repo.commit(message, user, opts.get('date'), match,
>> + editor=editor)
>> + finally:
>> + if hasmq:
>> + repo.mq.checkapplied = saved
>
> I'm not entirely sold in the benefit in these cases where using with introduce a new indentation level.
>
Favor pattern consistency over minor style warts because it increases readability and comprehension confidence because it eliminates questions like, "why does all the code except here use a context manager for config overrides?"
I'm not a huge fan of introducing indentation either. But something tells me that you wouldn't notice if it were new code. If you do take exception to with..try, we should be talking about forbidding that pattern via the style checker.
More information about the Mercurial-devel
mailing list