Hi Maxim,
On Mon, 16 Dec 2024 at 11:53, Maxim Cournoyer <maxim.courno...@gmail.com> wrote:
>> +_info()
>> +{
>> + if [ "$(type -P info)" ]; then
>> + _msg "$1"
>> + else
>> + _msg "${WAR}Please install Info reader; see package 'info-reader'"
>> + _msg "$1"
>> + fi
>> +}
>
> It seems odd to me to "overload" _msg into _info that deals with some side
> effect; I'd rather see this conditional explicit at the message printing
> site.
It was to avoid the duplication of the exact same conditional with the
exact same message.
I do not have an opinion…
> Also, your test is testing for the empty string when info is not found,
> not the exist status, which is wrong.
Please note that the script already uses:
if [ "$(type -P pidof)" ]; then
if [ ! "$(pidof nscd)" ]; then
And I have only respected the same. :-)
> not the exist status, which is wrong. I think you meant something like:
>
> --8<---------------cut here---------------start------------->8---
> if type -P info >/dev/null then [...]; fi
> --8<---------------cut here---------------end--------------->8---
Well, I am not a Bash expert but I guess that’s the same result in
practise, no?
Both $() and "" used in tandem makes the test sound, from my
understanding.
> But this got me curious again... could we instead automate the
> installation of info post-installation?
It appears to me unrelated to this change at hand. :-)
> If yes, we should also automate
> the installation of glibc-locales, using prompts that the user can
> accept or decline like for the other configuration choices.
Yeah why not, but let open another issue for tracking that, because
that’s not necessary straightforward since it’s on the top of different
distros.
Cheers,
simon
