Re: [PATCH] handle http urls with query string ("?foo") correctly

["David ‘Bombe’ Roden" <bombe@pterodactylus.net>]

[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]

On Thursday 05 June 2008 09:15:01 Johannes Schindelin wrote:

> And of course, you usually sign off your patches.

As soon as there are no more objections, I will. :)


> How about
>
> 	strbuf_addf(&suffix_buffer, "/objects/%c%c/%s",
> 		hex[0], hex[1], hex + 2);
>
> ?

Now that you mention it... ;) Changed.


> And this line wrapped, like so?
>
> 	url = transform_url(obj_req->repo->base,
> 		strbuf_detach(&suffix_buffer, NULL));

I usually don't do line-wrapping because all my terminals are wide enough but 
I might just make an exception here. :)


> Oh, but maybe it makes more sense to let transform_url() work on the
> strbuf itself?

Then I would have to rewrite more unrelated stuff because everything else in 
those files still relies on char*. I'd rather not meddle with that for now; I 
agree that at some point it should all be rewritten to use strbuf (more or 
less) exclusively. :)


> Junio usually prefers assignments outside of the if() statement.

Can do.


> But another thing is more pressing: obviously, there were reasons to quote
> the ref names with this funny URL encoding (%45%75%6E%6E%79), and you
> effectively undid that change.

Hmm, yes. Ref names should probably be url-escaped.


> > +#include <stddef.h>
> Why?

For ptrdiff_t. Yes, I know that the offset between the start of a URL and the 
question mark will never really exceed the 32-bit range and it's also highly 
unlikely that the two pointers in comparison differ in the upper 32 bits but 
as I'm actually calculating the difference between two pointers ptrdiff_t was 
the logical choice over int.


> Dscho

	David

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]