Patchwork ui: small cleanup in suboption parsing

login
register
mail settings
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

Pierre-Yves David - Dec. 7, 2015, 10:58 p.m.
# 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.
Martin von Zweigbergk - Dec. 8, 2015, 4:51 a.m.
Thanks, pushed to the clowncopter.
Yuya Nishihara - Dec. 8, 2015, 12:33 p.m.
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".
Augie Fackler - Dec. 8, 2015, 3:26 p.m.
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
Yuya Nishihara - Dec. 11, 2015, 1:38 p.m.
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
Pierre-Yves David - Dec. 11, 2015, 1:47 p.m.
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