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

From: David ‘Bombe’ Roden <bombe@pterodactylus.net>
Date: Jun 5, 2008, 08:29 UTC
Message-ID: <200806051029.22580.bombe@pterodactylus.net>
In-Reply-To: <alpine.DEB.1.00.0806050758210.21190@racer>

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. :)

Show 6 quoted lines

> 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