Patchwork [3,of,4,V2] ui: use the new configoverride context manager instead of saving/restoring

login
register
mail settings
Submitter Laurent Charignon
Date May 14, 2015, 12:26 a.m.
Message ID <cb2f0824776ea5d96481.1431563166@lcharignon-mbp.local>
Download mbox | patch
Permalink /patch/9062/
State Accepted
Headers show

Comments

Laurent Charignon - May 14, 2015, 12:26 a.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@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.
Pierre-Yves David - May 14, 2015, 12:42 a.m.
On 05/13/2015 05:26 PM, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@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.
Gregory Szorc - May 14, 2015, 1:29 a.m.
> On May 13, 2015, at 17:42, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> 
> 
>> On 05/13/2015 05:26 PM, Laurent Charignon wrote:
>> # HG changeset patch
>> # User Laurent Charignon <lcharignon@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.
Laurent Charignon - May 14, 2015, 3:36 a.m.
On 5/13/15, 6:29 PM, "Gregory Szorc" <gregory.szorc@gmail.com> wrote:

>
>
>> On May 13, 2015, at 17:42, Pierre-Yves David
>><pierre-yves.david@ens-lyon.org> wrote:
>> 
>> 
>> 
>>> On 05/13/2015 05:26 PM, Laurent Charignon wrote:
>>> # HG changeset patch
>>> # User Laurent Charignon <lcharignon@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.

From the reception of these changes, I feel like we would be better off
with continuing to do what we did before.
I will send a V3 without the context manager.
Pierre-Yves David - May 14, 2015, 5:23 a.m.
On 05/13/2015 08:36 PM, Laurent Charignon wrote:
>
>
> On 5/13/15, 6:29 PM, "Gregory Szorc" <gregory.szorc@gmail.com> wrote:
>
>>
>>
>>> On May 13, 2015, at 17:42, Pierre-Yves David
>>> <pierre-yves.david@ens-lyon.org> wrote:
>>>
>>>
>>>
>>>> On 05/13/2015 05:26 PM, Laurent Charignon wrote:
>>>> # HG changeset patch
>>>> # User Laurent Charignon <lcharignon@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.
>
>  From the reception of these changes, I feel like we would be better off
> with continuing to do what we did before.
> I will send a V3 without the context manager.

This seems wise, we have access to some new syntax and utility with 2.6. 
However it will take some to time to figure out how the project want to 
use them. You probably do not want ot get delayed until then.

Patch

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
 
     if parent.node() != nullid:
         desc = "changes to '%s'" % parent.description().split('\n', 1)[0]
@@ -568,15 +567,14 @@ 
                 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)
-                    return repo.commit(message, 'shelve@localhost',
+                with repo.ui.configoverride('phases', 'new-commit',
+                                            phases.secret):
+                    try:
+                        return repo.commit(message, 'shelve@localhost',
                                        opts.get('date'), match)
-                finally:
-                    repo.ui.restoreconfig(backup)
-                    if hasmq:
-                        repo.mq.checkapplied = saved
+                    finally:
+                        if hasmq:
+                            repo.mq.checkapplied = saved
 
             tempopts = {}
             tempopts['message'] = "pending changes temporary commit"
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1893,12 +1893,8 @@ 
                     if not streamreqs - self.supportedformats:
                         self.stream_in(remote, streamreqs)
 
-        quiet = self.ui.backupconfig('ui', 'quietbookmarkmove')
-        try:
-            self.ui.setconfig('ui', 'quietbookmarkmove', True, 'clone')
+        with self.ui.configoverride('ui', 'quietbookmarkmove', True, 'clone'):
             ret = exchange.pull(self, remote, heads).cgresult
-        finally:
-            self.ui.restoreconfig(quiet)
         return ret
 
     def pushkey(self, namespace, key, old, new):