[PATCH 3 of 5 paths v3] ui.paths: filter config options containing "." (BC)

[Gregory Szorc]

On Sun, Mar 15, 2015 at 12:55 PM, Matt Mackall <mpm at selenic.com> wrote:

> On Sat, 2015-03-14 at 22:18 -0700, Gregory Szorc wrote:
> > On Wed, Mar 11, 2015 at 3:55 PM, Matt Mackall <mpm at selenic.com> wrote:
> >
> > > I agree that the odds of people having foo.bar paths is probably 100%:
> > > it is after all how DNS names are structured. Unfortunately it's also
> > > how our config namespace is structured.
> > >
> >
> > I think it is important to distinguish between option names, option
> values,
> > and command line arguments.
> >
> > For option values and command line arguments, absolutely we'll have
> periods
> > because of hostnames. However, I believe our logic there is to first
> > attempt to parse those values as URLs, then fall back to a
> filesystem-local
> > path if a scheme isn't present. (Follow-up idea: have Mercurial store
> URLs
> > in more situations because bare strings are more ambiguous.)
>
> $ hg push selenic.com
> pushing to selenic.com
> abort: repository selenic.com not found!
>
> No, we treat bare hostnames as local paths.
>
> But I think people will actually have hostname-like entries as path
> _keys_, not values (where they won't actually work). And then use those
> path keys on the command line. For instance, I can imagine a project
> split between foo.org (open source) and foo.com (with commercial bits)
> and someone having both URLs in their ~/.hgrc. Or perhaps the domain is
> integral to the verbal identity like "healthcare.gov" (you'd never refer
> to it as just "healthcare" or "healthcaregov"). Picture a web design
> shop that's deploying to its dozens of clients' sites by domain name...
>
> > If we limit discussion to option values, I'm still not convinced
> supporting
> > periods is important.
> >
> >
> > > > I think a clean, one-time break is better.
> > >
> > > I greatly prefer zero-time breaks, but I don't have a great alternative
> > > suggestion. We do have a bit of precedent in the [auth] section though.
> > > Perhaps we can do:
> > >
> > > [paths]
> > > old.style = some/path
> > > new.style.path = some/other/pass
> > > new.style.option = foo
> > > bogus.option = foo
> > >
> > > Here, hg paths would list:
> > >
> > > old.style
> > > new.style
> > > bogus.option
> > >
> > > (..and we'd also change clone to make the default path setting
> > > "default.path".)
> > >
> >
> > This is an interesting idea. I like how it avoids ambiguity between
> whether
> > an entry is a path or isn't. However, it still has concerns:
> >
> > * Long-term Mercurial users now have to convert to a new config style to
> > use advanced options.
>
> Yeah, but I think that's pretty minor.
>
> > * Potential for collision between user-defined path names and per-path
> > options.
>
> I think the potential only exists for paths of the form "foo.bar.path",
> which signals "hey, we have a path with options named foo.bar". So if
> you happen to want to name something with a ".path" suffix, you'll now
> have to name it ".path.path". Not perfect, but not fatal.
>
> > I imagine a user will see they can add ".pushrev" to a path and produce a
> > snippet like:
> >
> > [paths]
> > central = https://hg.mozilla.org/mozilla-central
> > central.pushrev = .
> >
> > Without the ".path", this situation is ambiguous. Do we then have to
> > examine common option prefixes and do some kind of disambiguation to
> figure
> > out the user's intent?
>
> Yeah, this won't work in my scheme, it'll define two paths. A path X is
> only newstyle with options if X.path is defined. So any examples of
> central.pushrev would need to include central.path.
>
> >  What about something like this:
> >
> > [paths]
> > repo.prod = https://prod.hg.example.com/repo
> > repo.prod.pushrev = .
> > repo.dev = https://dev.hg.example.com/repo
> >
> > I still think unsupported periods in path names is looking pretty
> > reasonable. So does separate [path.x] sections.
>
> What's unsupported periods again? That's no dots in path keys?
>

Dots in path keys.


> I don't think that's going to work. I don't know that path.x sections
> work either, because the config format actually already treats dots as
> hierarchy separators and [foo] is just a special case for the tree top
> level. In other words [foo]bar.x and [foo.bar]x are both aliases for
> foo.bar.x.
>

I'm not so sure about that. I *think* config.config is preserving all
content in [SECTION]. I don't see anywhere where dots are getting collapsed.

(Pdb) import config
(Pdb) conf = config.config()
(Pdb) conf.parse('test', '[path.foo]\nbar = True')
(Pdb) pp conf._data
{'path.foo': {'bar': 'True'}}

I think passing the sections list down to the ui.paths constructor will
work. I did something similar in the first version of this patch series (I
was foolishly using ":" as the delimiter - "[path:foo]") and it seemed to
work fine.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150316/ce101d35/attachment.html>