Hi,
On Sat, 7 Mar 2009, Tay Ray Chuan wrote:
> In addition, ''quote_ref_url'' inserts a slash between the base URL and
> remote ref path only if needed. Previously, this insertion wasn't
> contingent on the lack of a separating slash.
>
> Signed-off-by: Tay Ray Chuan <rctay89@xxxxxxxxx>
> Acked-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
I would prefer to give my ACK explicitely... :-)
> http.c | 29 ++++++++++-------------------
> 1 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/http.c b/http.c
> index cdedeb6..9de4130 100644
> --- a/http.c
> +++ b/http.c
> @@ -577,31 +577,22 @@ static inline int hex(int v)
>
> static char *quote_ref_url(const char *base, const char *ref)
> {
> + struct strbuf buf = STRBUF_INIT;
> const char *cp;
> - char *dp, *qref;
> - int len, baselen, ch;
> + int ch;
> +
> + strbuf_addstr(&buf, base);
> + if (strcmp(base+strlen(base)-1, "/") && strcmp(ref, "/"))
> + strbuf_addstr(&buf, "/");
I would not have scratched my head that much if it read like this:
if (buf.len && buf.buf[buf.len - 1] != '/' && *ref != '/')
strbuf_addch(&buf, '/');
> for (cp = ref; (ch = *cp) != 0; cp++) {
> - if (needs_quote(ch)) {
> - *dp++ = '%';
> - *dp++ = hex((ch >> 4) & 0xF);
> - *dp++ = hex(ch & 0xF);
> - }
> + if (needs_quote(ch))
> + strbuf_addf(&buf, "%%%02x", ch);
> else
> - *dp++ = ch;
> + strbuf_addch(&buf, *cp);
> }
Seems as if you could remove even the curly brackets here.
Other than that, it indeed looks like an ACK from me...
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
