Patchwork [7,of,7,path-configs] ui.paths: accept multiple arguments to getpath()

login
register
mail settings
Submitter Gregory Szorc
Date Feb. 8, 2015, 1:13 a.m.
Message ID <f68fa5071815f89a369d.1423358023@vm-ubuntu-main.gateway.sonic.net>
Download mbox | patch
Permalink /patch/7768/
State Changes Requested
Headers show

Comments

Gregory Szorc - Feb. 8, 2015, 1:13 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1423350425 28800
#      Sat Feb 07 15:07:05 2015 -0800
# Node ID f68fa5071815f89a369dc55cc987e852344a07f5
# Parent  327b209f4ef203fd5d9bedb2527b8bfdc9edad3f
ui.paths: accept multiple arguments to getpath()

It is a common pattern to do something like
|ui.expandpath(dest or 'default-push', dest or 'default')| This is
essentially multiple fallback, albeit implemented in a manner that is
hard to read and understand. Since we're reinventing the paths APIs,
let's support a sane API for multi-query.
Sean Farley - Feb. 8, 2015, 9:02 a.m.
Gregory Szorc writes:

> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1423350425 28800
> #      Sat Feb 07 15:07:05 2015 -0800
> # Node ID f68fa5071815f89a369dc55cc987e852344a07f5
> # Parent  327b209f4ef203fd5d9bedb2527b8bfdc9edad3f
> ui.paths: accept multiple arguments to getpath()
>
> It is a common pattern to do something like
> |ui.expandpath(dest or 'default-push', dest or 'default')| This is
> essentially multiple fallback, albeit implemented in a manner that is
> hard to read and understand. Since we're reinventing the paths APIs,
> let's support a sane API for multi-query.

This series is extremely interesting. I am still digesting it but plan
to give it more attention tomorrow. I already like the fact that you
wrote doc strings and tests :-D
Gregory Szorc - Feb. 8, 2015, 8:51 p.m.
Please drop this one patch and only this one patch from the series. I'm
still implementing features built on top of this API and the more I
implement, the more I'm learning about what kind of API we really need for
getpath(). I originally intended this *names support to be used for
"default" lookup. I now feel that default lookup should be baked into this
API, rather than names passed by callers. This is a bit hard to explain
without code. The code is coming...

On Sat, Feb 7, 2015 at 5:13 PM, Gregory Szorc <gregory.szorc@gmail.com>
wrote:

> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1423350425 28800
> #      Sat Feb 07 15:07:05 2015 -0800
> # Node ID f68fa5071815f89a369dc55cc987e852344a07f5
> # Parent  327b209f4ef203fd5d9bedb2527b8bfdc9edad3f
> ui.paths: accept multiple arguments to getpath()
>
> It is a common pattern to do something like
> |ui.expandpath(dest or 'default-push', dest or 'default')| This is
> essentially multiple fallback, albeit implemented in a manner that is
> hard to read and understand. Since we're reinventing the paths APIs,
> let's support a sane API for multi-query.
>
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -568,9 +568,9 @@ class ui(object):
>          """Return repository location relative to cwd or from [paths]"""
>          if util.hasscheme(loc) or os.path.isdir(os.path.join(loc, '.hg')):
>              return loc
>
> -        p = self.paths.getpath(loc, default=default)
> +        p = self.paths.getpath(loc, default)
>          if p:
>              return p.url
>          else:
>              return loc
> @@ -1007,22 +1007,23 @@ class paths(object):
>              if path.name == key:
>                  return path
>          raise KeyError('path not known: %s' % key)
>
> -    def getpath(self, name, default=None):
> -        """Return a ``path`` for the specified name.
> +    def getpath(self, *names):
> +        """Return a ``path`` from a name.
>
> -        Returns None if the specified path or the default path was not
> -        found.
> +        If additional arguments are specified, we will search for those
> +        names as well. The first name with a match will be returned.
> +
> +        If no matches are found, None is returned.
>          """
> -        try:
> -            return self[name]
> -        except KeyError:
> -            if default is not None:
> -                try:
> -                    return self[default]
> -                except KeyError:
> -                    pass
> +        for name in names:
> +            if not name:
> +                continue
> +            try:
> +                return self[name]
> +            except KeyError:
> +                pass
>
>          return None
>
>  class path(object):
>
Sean Farley - Feb. 8, 2015, 9:46 p.m.
Gregory Szorc writes:

> Please drop this one patch and only this one patch from the series. I'm
> still implementing features built on top of this API and the more I
> implement, the more I'm learning about what kind of API we really need for
> getpath(). I originally intended this *names support to be used for
> "default" lookup. I now feel that default lookup should be baked into this
> API, rather than names passed by callers. This is a bit hard to explain
> without code. The code is coming...

Here is something else I'd like to get into core:

- given a remote repo object (or a string!) return the alias

For example, given 'https://sean@smf.io/hg' return 'smf' (which is a
path alias in my hgrc)

- compare a scheme with a uri

If I define 'smf://' to be 'https://sean@smf.io/' then I'd want a way to
compare 'smf://hg' == 'https://sean@smf.io/hg'

Patch

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -568,9 +568,9 @@  class ui(object):
         """Return repository location relative to cwd or from [paths]"""
         if util.hasscheme(loc) or os.path.isdir(os.path.join(loc, '.hg')):
             return loc
 
-        p = self.paths.getpath(loc, default=default)
+        p = self.paths.getpath(loc, default)
         if p:
             return p.url
         else:
             return loc
@@ -1007,22 +1007,23 @@  class paths(object):
             if path.name == key:
                 return path
         raise KeyError('path not known: %s' % key)
 
-    def getpath(self, name, default=None):
-        """Return a ``path`` for the specified name.
+    def getpath(self, *names):
+        """Return a ``path`` from a name.
 
-        Returns None if the specified path or the default path was not
-        found.
+        If additional arguments are specified, we will search for those
+        names as well. The first name with a match will be returned.
+
+        If no matches are found, None is returned.
         """
-        try:
-            return self[name]
-        except KeyError:
-            if default is not None:
-                try:
-                    return self[default]
-                except KeyError:
-                    pass
+        for name in names:
+            if not name:
+                continue
+            try:
+                return self[name]
+            except KeyError:
+                pass
 
         return None
 
 class path(object):