[PATCH 3 of 3] mq: remove leading slash from url when qimporting

[Brodie Rao]

On Sat, Apr 30, 2011 at 2:19 AM, Idan Kamara <idankk86 at gmail.com> wrote:
> On Sat, Apr 30, 2011 at 11:07 AM, Brodie Rao <brodie at bitheap.org> wrote:
>>
>> On Apr 29, 2011, at 9:32 PM, Idan Kamara wrote:
>>
>>> # HG changeset patch
>>> # User Idan Kamara <idankk86 at gmail.com>
>>> # Date 1304105557 -10800
>>> # Node ID 2e06c5551499416955dc1e77b6bfb76f144875d6
>>> # Parent  72f1f40451cb4ec16a268f233c0ceaee98f61de7
>>> mq: remove leading slash from url when qimporting
>>>
>>> When trying to qimport a url that ends with a slash
>>> os.path.basename would return an empty string. So
>>> when seeing a leading slash, remove it and infer the
>>> patch name from the remaining url.
>>>
>>> e.g. `hg qimport http://paste.pocoo.org/raw/xxx/`
>>> deduced '.' to be the patch name. Now we'll deduce 'xxx'.
>>>
>>> We could probably remove all leading slashes but this
>>> is the more common case.
>>
>> Some questions:
>>
>> - What if I run "hg qimport http://paste.pocoo.org/raw/xxx//"?
>
> Surely there are more complicated cases where the logic for inferring the
> patch name from the url will fail, I'm just not sure we want to try to do
> so. Maybe we can suggest the user to use --name.
> Like I said we could strip all leading slashes for this particular scenario,
> but I think most urls either end with a single slash or no slash at all.

Thinking about this some more, we should probably just abort with an
error if we're given a URL without a patch name specified and we can't
infer the patch name. curl does this:

$ curl -O http://paste.pocoo.org/raw/xxx/
curl: Remote file name has no length!
curl: try 'curl --help' or 'curl --manual' for more information

wget will give you index.html, but that probably doesn't make sense
for us to do.

>>
>> - Why aren't we using the url class to handle parsing here?
>
> I did contemplate that, but does it offer any advantage here? (it doesn't
> strip leading slashes for example)
>
>>
>> - What about a test?
>
> Is there an easier way other than starting a `hg serve` and importing a
> patch from it?
>
>>>
>>> diff -r 72f1f40451cb -r 2e06c5551499 hgext/mq.py
>>> --- a/hgext/mq.py       Fri Apr 29 22:21:13 2011 +0300
>>> +++ b/hgext/mq.py       Fri Apr 29 22:32:37 2011 +0300
>>> @@ -1821,6 +1821,8 @@
>>>                except (OSError, IOError):
>>>                    raise util.Abort(_("unable to read file %s") %
>>> filename)
>>>                if not patchname:
>>> +                    if filename.endswith('/'):
>>> +                        filename = filename[:-1]
>>>                    patchname = normname(os.path.basename(filename))
>>>                self.check_reserved_name(patchname)
>>>                checkfile(patchname)
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel at selenic.com
>>> http://selenic.com/mailman/listinfo/mercurial-devel