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

[Idan Kamara]

On Tuesday, May 17, 2011, Brodie Rao <brodie at bitheap.org> wrote:
> 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.

Yeah but urls with leading slashes are pretty common, aborting will be
annoying. I think the least we can do is try to infer the name after
removing all leading slashes (maybe even give a default name if we
still fail afterwards?).

> 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
>