On Sun, Feb 28, 2016 at 05:44:18PM +0100, Ludovic Courtès wrote:
> Sorry for the delay, and thanks for the useful patch!
No problem. :)
> I gather ‘svn’ doesn’t honor ‘http_proxy’ in the first place, right?
> That would have simplified things.
It really would, but for some reason that's not a feature. Well, it was for one
or two releases but that was unintentional and fixed.
> Also, what about ‘https_proxy’? (This could be left for a subsequent
> patch, if you prefer.)
I'd have to parse the subversion URL and figure out if its scheme is HTTPS as I
don't think Subverion has a https-proxy-* set of variables.
> > + (define proxy-config
> > + (if (current-http-proxy)
> > + (and-let* ((proxy-uri (string->uri (current-http-proxy)))
> > + (proxy-host (uri-host proxy-uri))
> > + (proxy-port (number->string (uri-port proxy-uri)))
> > + (config-host "servers:global:http-proxy-host=")
> > + (config-port "servers:global:http-proxy-port="))
> > + `("--config-option" ,(string-append config-host proxy-host)
> > + "--config-option" ,(string-append config-port proxy-port)))
> > + '()))
>
> I would suggest using an explicit (getenv "http_proxy") instead of
> (current-http-proxy) since the latter is specific to the (web …)
> modules, which are not involved here.
>
> Also, I’m not a fan of SRFI-2. :-) So I’d write it as:
>
> (match (getenv "http_proxy")
> (#f '())
> (uri-string
> (let* ((uri (string->uri uri-string))
> (host (uri-host uri))
> (port (uri-port uri)))
> (list (string-append
> "--config-option=servers:global:http-proxy-host="
> host)
> (string-append
> "--config-option=servers:global:http-proxy-port="
> (number->string port))))))
>
> Probably with something like:
>
> (unless uri
> (format (current-error-port) "invalid HTTP proxy URI: ~s~%"
> uri-string)
> (exit 1))
Interesting!
> Could you send an updated patch?
Sure, I'll do it hopefully this week.
> Thanks!
>
> Ludo’.
Cheers,
Jookia.
