Patchwork localrepo: make it possible to pass multiple path elements to join and wjoin

login
register
mail settings
Submitter Angel Ezquerra
Date Aug. 29, 2014, 3:13 p.m.
Message ID <1ab777ff5c4d58772ad6.1409325223@angels-macbook-pro.local>
Download mbox | patch
Permalink /patch/5624/
State Accepted
Headers show

Comments

Angel Ezquerra - Aug. 29, 2014, 3:13 p.m.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1409239385 -7200
#      Thu Aug 28 17:23:05 2014 +0200
# Node ID 1ab777ff5c4d58772ad6d849e6d94be1e4314515
# Parent  bdc0e04df243d3995c7266bf7d138fddd0449ba6
localrepo: make it possible to pass multiple path elements to join and wjoin

This makes join and wjoin behave in the same way as os.path.join. That is, it
makes it possible to pass more than one path element to them.

Note that the first path element is still required, as it was before this patch,
and as is the case for os.path.join.
Pierre-Yves David - Aug. 29, 2014, 3:48 p.m.
On 08/29/2014 05:13 PM, Angel Ezquerra wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra@gmail.com>
> # Date 1409239385 -7200
> #      Thu Aug 28 17:23:05 2014 +0200
> # Node ID 1ab777ff5c4d58772ad6d849e6d94be1e4314515
> # Parent  bdc0e04df243d3995c7266bf7d138fddd0449ba6
> localrepo: make it possible to pass multiple path elements to join and wjoin

Sounds good to me. This is now pushed to the clowncopter.
Angel Ezquerra - Aug. 29, 2014, 4:07 p.m.
On Fri, Aug 29, 2014 at 5:54 PM, Kevin Bullock
<kbullock+mercurial@ringworld.org> wrote:
> On Aug 29, 2014, at 5:13 PM, Angel Ezquerra <angel.ezquerra@gmail.com> wrote:
>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra@gmail.com>
>> # Date 1409239385 -7200
>> #      Thu Aug 28 17:23:05 2014 +0200
>> # Node ID 1ab777ff5c4d58772ad6d849e6d94be1e4314515
>> # Parent  bdc0e04df243d3995c7266bf7d138fddd0449ba6
>> localrepo: make it possible to pass multiple path elements to join and wjoin
>>
>> This makes join and wjoin behave in the same way as os.path.join. That is, it
>> makes it possible to pass more than one path element to them.
>>
>> Note that the first path element is still required, as it was before this patch,
>> and as is the case for os.path.join.
>
> Is there a use case for this? Will anyone use the extra components?

I have a couple of extensions where I do:

repo.join(os.path.join('foo', 'bar', 'baz'))

Also, it seems that it would be nice to have the same behavior as os.path.join.

Cheers,

Angel
Mads Kiilerich - Aug. 29, 2014, 8:46 p.m.
On 08/29/2014 06:07 PM, Angel Ezquerra wrote:
>> Is there a use case for this? Will anyone use the extra components?
> I have a couple of extensions where I do:
>
> repo.join(os.path.join('foo', 'bar', 'baz'))
>
> Also, it seems that it would be nice to have the same behavior as os.path.join.

Without any core usage of it and without any comments, how can we expect 
it to stay that way?

Anyway, it seems weird if your extensions are the only places where this 
is relevant. I would assume there would be some places in core Mercurial 
that could benefit from this as well.

/Mads
Angel Ezquerra - Aug. 30, 2014, 6:24 a.m.
On Fri, Aug 29, 2014 at 10:46 PM, Mads Kiilerich <mads@kiilerich.com> wrote:
> On 08/29/2014 06:07 PM, Angel Ezquerra wrote:
>>>
>>> Is there a use case for this? Will anyone use the extra components?
>>
>> I have a couple of extensions where I do:
>>
>> repo.join(os.path.join('foo', 'bar', 'baz'))
>>
>> Also, it seems that it would be nice to have the same behavior as
>> os.path.join.
>
>
> Without any core usage of it and without any comments, how can we expect it
> to stay that way?

Would you like me to add some comment or docstring to the join and
wjoin functions, explaining the new capability? Actually I think it is
easier to explain what those methods do now that they would behave the
same as os.path.join...

> Anyway, it seems weird if your extensions are the only places where this is
> relevant. I would assume there would be some places in core Mercurial that
> could benefit from this as well.
>
> /Mads

Actually the reason I wrote this patch in the first place is because I
found myself doing:

   os.path.join(repo.join('subs', path)

while writing a patch for core mercurial. I'm sure there are other
places were this would be useful.

Cheers,

Angel

Patch

diff -r bdc0e04df243 -r 1ab777ff5c4d mercurial/localrepo.py
--- a/mercurial/localrepo.py	Wed Aug 27 18:35:34 2014 +0200
+++ b/mercurial/localrepo.py	Thu Aug 28 17:23:05 2014 +0200
@@ -745,11 +745,11 @@ 
         # if publishing we can't copy if there is filtered content
         return not self.filtered('visible').changelog.filteredrevs
 
-    def join(self, f):
-        return os.path.join(self.path, f)
+    def join(self, f, *insidef):
+        return os.path.join(self.path, f, *insidef)
 
-    def wjoin(self, f):
-        return os.path.join(self.root, f)
+    def wjoin(self, f, *insidef):
+        return os.path.join(self.root, f, *insidef)
 
     def file(self, f):
         if f[0] == '/':