| Submitter | Pierre-Yves David |
|---|---|
| Date | Dec. 7, 2015, 10:58 p.m. |
| Message ID | <4259ca7d7d1eefd25f92.1449529109@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/11914/ |
| State | Accepted |
| Headers | show |
Comments
Thanks, pushed to the clowncopter.
On Mon, 07 Dec 2015 14:58:29 -0800, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1449528679 28800 > # Mon Dec 07 14:51:19 2015 -0800 > # Node ID 4259ca7d7d1eefd25f926e3ebff52b9d48d4344c > # Parent a9ab664083bff058206777783da85bee0a20c74e > # EXP-Topic cleanup.pushattr > # Available At http://hg.netv6.net/marmoute-wip/mercurial/ > # hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r 4259ca7d7d1e > ui: small cleanup in suboption parsing > > Anything after the first ':' is expected to be a suboption name. The old code is > correct but it: > - use 'rsplit' for no good reason (same as 'split' unless 'maxsplit' is used), > - splits multiples time even if only the first value is of interest. > > We move to "split(':', 1)" because it is doing exactly what we need. > > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -319,11 +319,11 @@ class ui(object): > if v is None: > return None > if not os.path.isabs(v) or "://" not in v: > src = self.configsource(section, name, untrusted) > if ':' in src: > - base = os.path.dirname(src.rsplit(':')[0]) > + base = os.path.dirname(src.split(':', 1)[0]) > v = os.path.join(base, os.path.expanduser(v)) This is wrong and unrelated to the suboption. ui.configpath() resolves a path relative to the hgrc file. Here src is "/path/to/hgrc:lineno" and rsplit(':')[0] strips off ":lineno".
On Tue, Dec 08, 2015 at 04:51:26AM +0000, Martin von Zweigbergk wrote: > Thanks, pushed to the clowncopter. Per Yuya's review, I'm going to drop this from the clowncopter for now. > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > https://selenic.com/mailman/listinfo/mercurial-devel
On Tue, 8 Dec 2015 10:26:58 -0500, Augie Fackler wrote: > On Tue, Dec 08, 2015 at 04:51:26AM +0000, Martin von Zweigbergk wrote: > > Thanks, pushed to the clowncopter. > > Per Yuya's review, I'm going to drop this from the clowncopter for now. Still included in the clowncopter. Should I drop or backout it? http://hg.netv6.net/clowncopter/log?rev=d64b678c2e4e
On 12/11/2015 01:38 PM, Yuya Nishihara wrote: > On Tue, 8 Dec 2015 10:26:58 -0500, Augie Fackler wrote: >> On Tue, Dec 08, 2015 at 04:51:26AM +0000, Martin von Zweigbergk wrote: >>> Thanks, pushed to the clowncopter. >> >> Per Yuya's review, I'm going to drop this from the clowncopter for now. > > Still included in the clowncopter. Should I drop or backout it? > > http://hg.netv6.net/clowncopter/log?rev=d64b678c2e4e I'll push a drop in the next 5 minutes >
Patch
diff --git a/mercurial/ui.py b/mercurial/ui.py --- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -319,11 +319,11 @@ class ui(object): if v is None: return None if not os.path.isabs(v) or "://" not in v: src = self.configsource(section, name, untrusted) if ':' in src: - base = os.path.dirname(src.rsplit(':')[0]) + base = os.path.dirname(src.split(':', 1)[0]) v = os.path.join(base, os.path.expanduser(v)) return v def configbool(self, section, name, default=False, untrusted=False): """parse a configuration element as a boolean
# HG changeset patch # User Pierre-Yves David <pierre-yves.david@fb.com> # Date 1449528679 28800 # Mon Dec 07 14:51:19 2015 -0800 # Node ID 4259ca7d7d1eefd25f926e3ebff52b9d48d4344c # Parent a9ab664083bff058206777783da85bee0a20c74e # EXP-Topic cleanup.pushattr # Available At http://hg.netv6.net/marmoute-wip/mercurial/ # hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r 4259ca7d7d1e ui: small cleanup in suboption parsing Anything after the first ':' is expected to be a suboption name. The old code is correct but it: - use 'rsplit' for no good reason (same as 'split' unless 'maxsplit' is used), - splits multiples time even if only the first value is of interest. We move to "split(':', 1)" because it is doing exactly what we need.