[PATCH 5 of 9 paths v2] ui: move URL and path detection into getpath()

Gregory Szorc gregory.szorc at gmail.com
Thu Mar 5 13:25:29 CST 2015


On Wed, Mar 4, 2015 at 11:47 AM, Matt Mackall <mpm at selenic.com> wrote:

> On Sun, 2015-03-01 at 13:50 -0800, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc at gmail.com>
> > # Date 1423427350 28800
> > #      Sun Feb 08 12:29:10 2015 -0800
> > # Node ID 0aaf05f23aae7fdc440412af2f9815daaa9c4123
> > # Parent  4d9ba0789bf42ad77c407c84b19a6486652730ed
> > ui: move URL and path detection into getpath()
> >
> > ui.expandpath() has code for recognizing URLs or local filesystem
> > paths. Our goal is to use path instances in more locations. Since many
> > call sites pass in arguments that could be URLs or filesystem paths,
> > we move this code into paths.getpath() so that more callers can be
> > switched to using the new API directly instead of ui.expandpath().
>
> It's weird to me that the logic for this is not in the path object
> contructor? The paths class has no more information and it seems
> unlikely we'll want to manually construct paths that subvert the logic?
>

I don't disagree with your opinions. I intentionally attempted to minimize
the amount of extra refactoring that went into this series. My thinking was
that it would maximize the chances for acceptance. I figured making things
sane could be deferred to once the world is using the new API.


> It's also a bit weird that this is implemented as member variables
> rather than trivial methods. It seems like we'd want to have things like
> path.url() and path.safeurl() (wrapping util.hidepassword).
>

I probably didn't make this clear enough, but my intent was for path
instances to be immutable. I figured having an internal representation of
some of the most widely used variables (namely the raw "loc" and the parsed
url) made sense. I suppose some of the lesser-used properties could be
defined as properties or trivial methods instead of as pre-generated
attributes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150305/1d15fd85/attachment.html>


More information about the Mercurial-devel mailing list