[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [patch] new variant of Dominique Leuenberger's libproxy p
Re: [Bug-wget] [patch] new variant of Dominique Leuenberger's libproxy patch
Thu, 19 Apr 2012 19:20:23 +0200
Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20120327 Thunderbird/11.0.1
On 19.04.2012 00:30, Ángel González wrote:
> Some minor comments:
> In line with the recent const discussion in this list, direct should be
> const char.
> The wget code generally uses C89 variable definition before statements,
> which would be nice to keep by this patch.
> The two asprintf() are just strdup() in disguise, so I'd use the later
> Moreover, they're both unneeded.
> As you moved the libproxy code above wget proxy code, it would now be
> for the pac file to return "do a direct connection", and for wget to try
> a proxy from http_proxy env variable (probably a default for
> non-libproxy aware
Don't you think it would be less confusing to the user to avoid mixing
libproxy handling with wget's own? Libproxy may get it's configuration
from various sources (kde, gnome settings, some pacrunner, not only the
standard environment variables from which various programs including
wget can get them), therefore I think separating the code even more
would be better. Maybe some user may have to explicitely bypass libproxy
on a binary distribution like most of them are (I'm in the lucky
position to use gentoo and compile libs and programs the way I want to
combine their features), for that I think a user option like provided in
the current patch (--no-libproxy, only effective if proxy turned on) may
fit. If some "..tp_proxy" variable is set to pac+http://... and wget is
called without libproxy support, it will simply complain about
unsupported scheme. Maybe that user message should be detailed a bit to
suggest using libproxy if compiled in.
> Not sure if httpproxy from .wgetrc should be honored in that case or not.
See above, my thoughts about mixing. Nevertheless, I think I extended
the libproxy option in the rc file as well
> A further enhacement would be having wget support multiple proxies, as
> is expected
> by libproxy ("If the first proxy fails, the second should be tried,
> etc..."), but just
> supporting the first proxy (as in openSUSE patch) seems a good enough
> change for
> the first addition of libproxy support.
I implemented at least finding a proxy to which wget can establish a
connection, out of the possibly larger list libproxy provides, I noticed
wget reuses that connection then. Is that ok?
Description: Text document