On Thu, Apr 1, 2010 at 10:27:55 +0200, Martin Pitt wrote:
> Hello Julien,
>
> Julien Cristau [2010-03-14 12:04 +0100]:
> > > + - Use shell built in "type" instead of external "which" to test for
> > > + programs.
> >
> > There's no guarantee that /bin/sh has a 'type' built-in, as far as I can
> > tell from SUSv3 and policy). I'd suggest command -v, but apparently
> > posh doesn't like that either, so I don't know.
>
> Hm, it's present in dash and bash, anyway. But if you like command -V
> better, I'm fine with that.
>
I'd like something which is guaranteed to work. "which" was, "type"
isn't...
Another way would be to run xrdb -help >/dev/null 2>&1 and test the
result of that.
> > > + - 30x11-common_xresources: Swap the order of tests to keep the most
> > > + unlikely (like "~/.Xresources exists") outside, to avoid running the
> > > + other tests (like "xrdb exists") on systems which don't use Xresources.
> > > +
> >
> > I'm not sure about this one. x11-common installs a file in
> > /etc/X11/Xresources, so you'll end up looking for xrdb on pretty much
> > every system regardless.
>
> Right, it's a minor case, if someone decides to remove
> /etc/X11/Xresources/x11-common. But I still think it's a tad less
> likely to be true than the existence of xrdb.
>
> > And then the reordering means you're looking for it twice (granted,
> > with 'type' the shell can cache the result of the first lookup, but
> > still).
>
> Hm, I don't understand? The second type is only done if
> [ -f "$USRRESOURCES" ], and ~/.Xresources should exist pretty seldomly
> these days?
>
we went from:
if has xrdb; then
if has sysresources; then
xrdb -merge sysresources
fi
if want userresources and has userresources; then
xrdb -merge userresources
fi
else
warn
fi
to
if has sysresources and has xrdb; then
xrdb -merge sysresources
fi
if want userresources and has userresources; then
if has xrdb; then
xrdb -merge userresources
else
warn
fi
fi
The changelog says this was to avoid testing for xrdb if ~/.Xresources
doesn't exist. But we'll keep testing for xrdb anyway, since we'll
pretty much always have /etc/X11/Xresources/. So you'll still run the
exact same tests.
Plus there's a behaviour change in the new version which only warns when
~/.Xresources exists and is enabled.
So I'm still not convinced this reordering provides any gain. Not a big
deal though.
Cheers,
Julien
Attachment:
signature.asc
Description: Digital signature