Steve Youngs <steve@xxxxxxxx> writes:
> * Zajcev Evgeny <zevlg@xxxxxxxxx> writes:
>
> > Steve Youngs <steve@xxxxxxxx> writes:
> >> * Zajcev Evgeny <lg@xxxxxxxx> writes:
> >> > * lisp/xwem-weather.el
>
> >> _*WAIT*_!! Don't do anything drastic like commit a --version-0
> >> on this yet. I have a some issues with these changes and I need
> >> some time to go over the code so the forthcoming arguments and
> >> fights we're gonna have over it will be more fulfilling. :-P
>
> > Haha, sure man, waiting for your comments ..
>
> OK, here they come.
>
> ,----
> | (eval-when-compile
> | (autoload 'url-retrieve "url"))
> `----
>
> I _strongly_ object to this. The reason I wrote xwem-weather the way
> I did was so that it could do the fetching _without_ using any
> external (to XWEM/XLIB) libraries.
>
> IMO this adds a dependency to XWEM that really is not worth the
> bother. Now, I know you are going to come back with "using url
> package means we can go through a proxy if needed"... true... BUT,
> for that to work, the luser must have W3 configured to do so.
>
> Would you like to take a guess at how big a number the subset of
> users who _must_ access the web via a proxy, _and_ use W3, _and_
> have W3 proxies configured right, _and_ use XWEM, _and DO NOT_
> have a transparent proxy where their firewall automatically routes
> all outgoing port 80 traffic through the proxy anyway? I'd bet my
> left one that the answer was lower than one.
>
> And even if there were a 1000 such users, I'd rather see xwem-weather
> handle proxy connections by itself instead of dragging in code that
> hasn't been updated since before you were born.
Hehe, i understand your complains, however they are not quite in place
.. xwem-weather *still* not depended on any external package. However
if you want it to use configured url library (for example to access
web via proxy) you can do it. There is new custom variable -
`xwem-weather-fetch-function' and two fetching functions that you
can use in the configuration:
o `xwem-weather-fetch-direct' - When you are having direct access to
the internet.
o `xwem-weather-fetch-with-url' - When you are having url library
installed and configured to access web.
By default fetcher is autoconfigurable, firstly it tries access web
directly and only then(if fails) tries to use url library.
and as you already got, lines
(eval-when-compile
(autoload 'url-retrieve "url"))
just shuts up compiler's warnings
As to url library code quality - it is very qualitative and work
perfectly, yes it is old, but old does not mean bad :)
>
> ,----
> | (defun xwem-weather-data-file ()
> | "Return filename where to store/read weather info."
> | (expand-file-name (concat "weather-" xwem-weather-station-id)
> | xwem-weather-data-directory))
> `----
>
> This isn't a biggie, and I really don't care either way, but why
> change from...
>
> ~/.xwem/ybbn
>
> to...
>
> ~/.xwem/weather-ybbn (using Brisbane, AU ID as an example)
>
> Seems to me that the `(concat "weather-"' is just fluff. Good idea to
> change it to a defun though. :-)
Haha, main reason is that i was questioning myself many times what
that files uuww, ybbn, uuee in the ~/.xwem directory .. i got answer
only when print out that files :), "weather" prefix will answer the
question in the future without need in printing files :)
>
> ,----
> | ;;;###autoload
> | (defun xwem-weather (&optional dockid dockgroup dockalign)
> | "Initialise the weather dock."
> | (interactive)
> |
> | (when (xwem-osd-p xwem-weather-osd)
> | (xwem-osd-destroy xwem-weather-osd))
> | (when (itimerp (get-itimer "xwem-weather-itimer"))
> | (delete-itimer (get-itimer "xwem-weather-itimer")))
> | (xwem-weather-display-temp 'force)
> | (unless (zerop xwem-weather-frequency)
> | (start-itimer "xwem-weather-itimer"
> | 'xwem-weather-update
> | xwem-weather-frequency
> | xwem-weather-frequency)))
> |
> | (defalias 'xwem-weather-init 'xwem-weather)
> `----
>
> What's the point of renaming `xwem-weather-init' to `xwem-weather' and
> making it interactive? This really isn't something a user needs to
> get confused over. If you really do want something interactive, how
> about...
To make xwem-weather to be a bit like xwem-pager, xwem-battery,
xwem-time, etc and yes i like `xwem-weather-init' name too, so i kept
the compatibility with defalias :)
>
> (defun xwem-weather-brute-force-reload ()
> "Kill your weather dock and reload a new one in a totally different
> spot on your systray. Normal people would never do this, but I guess
> you know what you are doing, so have at it, buddy."
> (interactive)
> (xwem-weather-init))
>
> > Actually i was planning to commit version-0, but was too drunk to do
> > that .. i simple forgot to specify --seal flag :)
>
> Dude, you're nuts! That last changeset of yours had quite a lot of
> shit in it. Some of it no doubt will be a bit destabilising. On a
> changeset like that you need to let it settle for a while before you
> seal it.
Haha, yeah, that is good idea, i suspect that osd changes especially
might cause problems ..
Thanks Steve for such interesting and contentful comments!! :)
--
XWEM - The (S)XEmacs Window Manager.
|