| Submitter | Kostia Balytskyi |
|---|---|
| Date | Nov. 22, 2016, 12:24 a.m. |
| Message ID | <a7c57b059190f02a4506.1479774298@dev1902.lla1.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/17675/ |
| State | Accepted |
| Headers | show |
Comments
> On Nov 21, 2016, at 7:24 PM, Kostia Balytskyi <ikostia@fb.com> wrote: > > # HG changeset patch > # User Kostia Balytskyi <ikostia@fb.com> > # Date 1479774146 28800 > # Mon Nov 21 16:22:26 2016 -0800 > # Node ID a7c57b059190f02a450667411d92d9a4862f6375 > # Parent 141b0d27e9e1e846215ead5314237536efc1a185 > ui: add configoverride context manager Queued this, thanks. > > I feel like this idea might've been discussed before, so please > feel free to point me to the right mailing list entry to read > about why it should not be done. > > We have a common pattern of the following code: > backup = ui.backupconfig(section, name) > try: > ui.setconfig(section, name, temporaryvalue, source) > do_something() > finally: > ui.restoreconfig(backup) > > IMO, this looks better: > with ui.configoverride({(section, name): temporaryvalue}, source): > do_something() > > Especially this becomes more convenient when one has to backup multiple > config values before doing something. In such case, adding a new value > to backup requires codemod in three places. > > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -7,6 +7,7 @@ > > from __future__ import absolute_import > > +import contextlib > import errno > import getpass > import inspect > @@ -1193,6 +1194,23 @@ class ui(object): > " update your code.)") % version > self.develwarn(msg, stacklevel=2, config='deprec-warn') > > + @contextlib.contextmanager > + def configoverride(self, overrides, source=""): > + """Context manager for temporary config overrides > + `overrides` must be a dict of the following structure: > + {(section, name) : value}""" > + backups = {} > + for (section, name), value in overrides.items(): > + backups[(section, name)] = self.backupconfig(section, name) > + self.setconfig(section, name, value, source) > + yield > + for __, backup in backups.items(): > + self.restoreconfig(backup) > + # just restoring ui.quiet config to the previous value is not enough > + # as it does not update ui.quiet class member > + if ('ui', 'quiet') in overrides: > + self.fixconfig(section='ui') > + > class paths(dict): > """Represents a collection of paths and their configs. > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Mon, 21 Nov 2016 16:24:58 -0800, Kostia Balytskyi wrote: > # HG changeset patch > # User Kostia Balytskyi <ikostia@fb.com> > # Date 1479774146 28800 > # Mon Nov 21 16:22:26 2016 -0800 > # Node ID a7c57b059190f02a450667411d92d9a4862f6375 > # Parent 141b0d27e9e1e846215ead5314237536efc1a185 > ui: add configoverride context manager > > I feel like this idea might've been discussed before, so please > feel free to point me to the right mailing list entry to read > about why it should not be done. > > We have a common pattern of the following code: > backup = ui.backupconfig(section, name) > try: > ui.setconfig(section, name, temporaryvalue, source) > do_something() > finally: > ui.restoreconfig(backup) > > IMO, this looks better: > with ui.configoverride({(section, name): temporaryvalue}, source): > do_something() > > Especially this becomes more convenient when one has to backup multiple > config values before doing something. In such case, adding a new value > to backup requires codemod in three places. > > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -7,6 +7,7 @@ > > from __future__ import absolute_import > > +import contextlib > import errno > import getpass > import inspect > @@ -1193,6 +1194,23 @@ class ui(object): > " update your code.)") % version > self.develwarn(msg, stacklevel=2, config='deprec-warn') > > + @contextlib.contextmanager > + def configoverride(self, overrides, source=""): > + """Context manager for temporary config overrides > + `overrides` must be a dict of the following structure: > + {(section, name) : value}""" > + backups = {} > + for (section, name), value in overrides.items(): > + backups[(section, name)] = self.backupconfig(section, name) > + self.setconfig(section, name, value, source) > + yield > + for __, backup in backups.items(): > + self.restoreconfig(backup) > + # just restoring ui.quiet config to the previous value is not enough > + # as it does not update ui.quiet class member > + if ('ui', 'quiet') in overrides: > + self.fixconfig(section='ui') Missed try-finally ?
Patch
diff --git a/mercurial/ui.py b/mercurial/ui.py --- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -7,6 +7,7 @@ from __future__ import absolute_import +import contextlib import errno import getpass import inspect @@ -1193,6 +1194,23 @@ class ui(object): " update your code.)") % version self.develwarn(msg, stacklevel=2, config='deprec-warn') + @contextlib.contextmanager + def configoverride(self, overrides, source=""): + """Context manager for temporary config overrides + `overrides` must be a dict of the following structure: + {(section, name) : value}""" + backups = {} + for (section, name), value in overrides.items(): + backups[(section, name)] = self.backupconfig(section, name) + self.setconfig(section, name, value, source) + yield + for __, backup in backups.items(): + self.restoreconfig(backup) + # just restoring ui.quiet config to the previous value is not enough + # as it does not update ui.quiet class member + if ('ui', 'quiet') in overrides: + self.fixconfig(section='ui') + class paths(dict): """Represents a collection of paths and their configs.
# HG changeset patch # User Kostia Balytskyi <ikostia@fb.com> # Date 1479774146 28800 # Mon Nov 21 16:22:26 2016 -0800 # Node ID a7c57b059190f02a450667411d92d9a4862f6375 # Parent 141b0d27e9e1e846215ead5314237536efc1a185 ui: add configoverride context manager I feel like this idea might've been discussed before, so please feel free to point me to the right mailing list entry to read about why it should not be done. We have a common pattern of the following code: backup = ui.backupconfig(section, name) try: ui.setconfig(section, name, temporaryvalue, source) do_something() finally: ui.restoreconfig(backup) IMO, this looks better: with ui.configoverride({(section, name): temporaryvalue}, source): do_something() Especially this becomes more convenient when one has to backup multiple config values before doing something. In such case, adding a new value to backup requires codemod in three places.