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.