Re: [PATCH] Add support for HTTP proxies

[Ludovic Courtès]

Hello!

Mark H Weaver <address@hidden> skribis:

> address@hidden (Ludovic Courtès) writes:

[...]

>>> +(define current-http-proxy (make-parameter 'auto))
>>> +
>>> +(define (choose-http-proxy)
>>> +  (let ((proxy (current-http-proxy)))
>>> +    (if (not (eq? proxy 'auto))
>>> +        proxy
>>> +        (let ((proxy (getenv "http_proxy")))
>>> +          (and (not (equal? proxy ""))
>>> +               proxy)))))
>>
>> What about just:
>>
>>   (make-parameter (getenv "http_proxy"))
>
> As Taylan pointed out, http_proxy="" is a widely supported convention
> beyond of our control.  However, I agree with you that internally, we
> should keep it simple and strict, and not support "" as a valid setting
> for (current-http-proxy).  So how about this:
>
>    (define current-http-proxy
>      (make-parameter (let ((proxy (getenv "http_proxy")))
>                        (and (not (equal? proxy ""))
>                             proxy))))

Looks good to me.

> I've attached an updated patch incorporating your suggestions, and
> adding documentation for 'current-http-proxy'.  I'm not sure if I want
> to bother documenting 'http-proxy-port?' et al.

Agreed.  Since it’s internal, I wonder if they should be prefixed with
‘%’, or kept really internal, with (web client) using @@ to access them.
The way you prefer.

> What do you think?

LGTM!

> address@hidden {Parameter} current-http-proxy

Perhaps {Scheme Parameter}?  (I don’t think there’s a precedent.)

Good, thanks for working on this!

Ludo’.