On Tue, Jun 04, 2019 at 03:59:21PM -0700, Matthew DeVore wrote: > > I think you'll find that -Wunused-function complains, though, if nobody > > is calling it. I wasn't sure if what you showed in the interdiff was > > meant to be final (I had to add a few other variable declarations to > > make it compile, too). > > Sorry, my last interdiff was a mess because I made a mistake during git rebase > -i. It was missing a call to has_reserved_char. Below is another diff that > fixes the problems: Ah, OK, that makes sense then (and keeping the function is obviously the right thing to do). > > That makes some sense, and I agree that readability is a good goal. Do > > we not need to be escaping colons in other URLs? Or are the strings > > you're generating not true by-the-book URLs? I'm just wondering if we > > could take this opportunity to improve the URLs we output elsewhere, > > too. > > The strings I'm generating are not URLs. Also, in http.c, we have to use : to > delimit a username and password: > > strbuf_addstr_urlencode(&s, proxy_auth.username, 1); > strbuf_addch(&s, ':'); > strbuf_addstr_urlencode(&s, proxy_auth.password, 1); > > I think this is dictated by libcurl and is not flexible. Right, that has to be a real colon because it's syntactically significant (but a colon in the username _must_ be encoded). That strbuf function doesn't really understand whole URLs, and it's up to the caller to assemble the parts. Anyway, we've veered off of your patch series enough. Yeah, it sounds like using strbuf's url-encoding is not quite what you want. -Peff
