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

[Gregory Szorc]

On Mon, Mar 9, 2015 at 8:10 PM, Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

>
>
> On 03/08/2015 12:54 PM, Gregory Szorc wrote:
>
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc at gmail.com>
>> # Date 1425243224 28800
>> #      Sun Mar 01 12:53:44 2015 -0800
>> # Node ID 0f9c6588afcbaaca41b8a5ff17d8969e7f6d157a
>> # Parent  57681bad997779da88778ff95254951c89250441
>> ui.paths: filter config options containing "." (BC)
>>
>> Upcoming patches will enable additional attributes to be defined
>> on a per-path basis. This will be done by creating config options
>> of the form "path.X" and "path.Y."
>>
>> This patch teaches the new paths API to assume options containing
>> "." are per-path attributes and not valid path names.
>>
>> This patch is technically backwards incompatible because nothing
>> was preventing people from using "." in path names. However, the
>> author feels this convention is not widespread enough to warrant
>> maintaining backwards compatibility. If backwards compatibility
>> is needed, we'll have to employ different functionality for
>> defining per-path attributes, such as separate config sections
>> per path. An earlier version of this patch series did feature
>> separate config sections per path. However, people felt that
>> adopting sub-options was a better approach since this is done
>> elsewhere (such as with merge tool definition).
>>
>
> Sorry to be late to the party, but I feel like this change is, as is, too
> user-hostile. I'm -certain- there is people out there using `foo.bar`
> paths, and just breaking them seems unfortunate.
>
> Can we get something so that `hg push foo.bar` still works if foo.bar
> exists? (possibly with a warning?) If a someone explicitly ask for foo.bar
> there is a good chance this is not an attributes and that we should give
> them what they asked for.
>
> We should probably also lists them in `hg path` if they do not match any
> known attributes. That would reduce script breakage.


I can make the case that half-support for `foo.bar` paths is even more
hostile than the proposed clean break.

With half-support for `foo.bar` paths, we run the risk of breaking people
with every future Mercurial upgrade. Any new attribute runs the risk of
colliding with a user's path. So now when users upgrade, they are left with
uncertainty as to whether their paths with "." will continue to work. And
it's not just core Mercurial upgrades either. What about extensions
providing their own path attributes? How does the core code know when an
option is a path versus an attribute used by an extension? (I reckon we'll
need to provide a mechanism for extensions to filter out certain
attributes.)

I think a clean, one-time break is better. Users can do a one-time
conversion of paths and be done with it. No surprises in the future. And as
a bonus it makes the code simpler and removes uncertainty about naming
collisions from future attribute additions. We can support the transition
by adding code that detects when the user requests a path with a "." that
matches a config option and abort with a message telling them of the
ambiguous scenario and that they should change their path names.

I think the audience impacted by this is small. I don't think a small
audience warrants additional complexity and ambiguity required by
half-support for paths with ".". FWIW, I'd be more inclined to support
different [path:foo.bar] sections than a hybrid mode: I really don't like
ambiguity in behavior.

Does that change your opinion at all? Anyone else want to weigh in?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150310/a8edf307/attachment.html>