Re: [PATCH v7 1/9] connect: document why we sometimes call get_port after get_host_and_port

["Torsten Bögershausen" <tboegi@web.de>]

On 05/22/2016 10:03 AM, Mike Hommey wrote:
> On Sun, May 22, 2016 at 08:07:05AM +0200, Torsten Bögershausen wrote:
>> On 22.05.16 01:17, Mike Hommey wrote:
>>> Signed-off-by: Mike Hommey <mh@glandium.org>
>>> ---
>>>   connect.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/connect.c b/connect.c
>>> index c53f3f1..caa2a3c 100644
>>> --- a/connect.c
>>> +++ b/connect.c
>>> @@ -742,6 +742,12 @@ struct child_process *git_connect(int fd[2], const char *url,
>>>   			transport_check_allowed("ssh");
>>>   			get_host_and_port(&ssh_host, &port);
>>>   
>>> +			/* get_host_and_port may not return a port even when
>>> +			 * there is one: In the [host:port]:path case,
>>> +			 * get_host_and_port is called with "[host:port]" and
>>> +			 * returns "host:port" and NULL.
>>> +			 * In that specific case, we still need to split the
>>> +			 * port. */
>> Is it worth to mention that this case is "still supported legacy" ?
> If it's worth mentioning anywhere, it seems to me it would start with
> urls.txt?
>
> Mike
>
I don't know.
urls.txt is for Git users, and connect.c is for Git developers.
urls.txt does not mention that Git follows any RFC when parsing the
URLS', it doesn't claim to be 100% compliant.
Even if it makes sense to do so, as many user simply expect Git to accept
RFC compliant URL's, and it makes the development easier, if there is an 
already
written specification, that describes all the details.
The parser is not 100% RFC compliant, one example:
- old-style usgage like "git clone [host:222]:~/path/to/repo are supported
To easy the live for developers, it could make sense why the code is as 
it is,
simply to avoid that people around the world suddenly run into problems,
when they upgrade the Git version.
So if there is a comment in the code, it could help new developers to 
understand
things easier.