Re: [Dragora-members] Qi 2.0rc10

[Matias Fonzo]

Hi Michael,

Thanks for taking the time in review Qi 2.0rc10! ;-)

El 2020-07-13 14:38, Michael Siegel escribió:
> Am 04.07.20 um 22:35 schrieb Matias Fonzo:
> > Qi 2.0rc10 for review, attached.
>
> Okay, I'll keep this concise. This is just what I found having a rather
> quick look over the source.
>
>   * L 46: Usage: $PROGRAM COMMAND [OPTION...] [FILE]...
>     * POSIX would also allow for simply saying "[OPTIONS]"[1]. I think
>       that's a good way to do it in this case.
>     * POSIX is not really clear on this, but I think it should rather 
> be
>       "[FILE...]".
>     * This should then also be adjusted in the manual page.

I changed it to [OPTIONS].  While [FILE]... prevails as I see nothing 
wrong with it, it is widely used in GNU tools.

>   * L 112, 380: printf "%s\\n"
>     * Wouldn't it be better to say: printf '%s\n'

Yes.  This was pointed by 
https://github.com/koalaman/shellcheck/wiki/SC1117 before.
I changed it to printf '%s\n' now.

>   * L 262–265: You might want to quote those substitutions, even though
>     they probably won't contain spaces. I mean, you're already doing it
>     in the assignment block right above that.

I consider it safe, but quoted now for consistency.

>   * L 323: outdir=${outdir}/${arch}
>     * Might want to quote the substitution.

True, better to be safe.

>   * L 1506: \unalias -a;                # Unset all possible aliases.
>     * Why is there a backslash before "unalias"?

To avoid invoking a possible alias of "unalias".

>     * Also, I'm not sure if that is needed because, usually aliases
>       don't apply when running scripts, IIRC. We'd need to verify that
>       in POSIX, mksh(1) and the Bash Reference Manual.

This would put off any possible declaration of aliases from the 
configuration file, I'm not sure if anyone will declare useful aliases 
there to be used in the recipe...

> Generally, I'd go through the code again and look for unquoted variable
> expansions and decide whether to better quote them or not. I mean, it 
> is
> more consistent to just quote every substitution, except for where you
> don't quote things on purpose, I think.

Agree.  I will take a look on the code for such cases.

> Also, maybe you could convert all usage of `echo' to `printf'. Maybe
> that would even be more convenient/practical in some cases.

I use echo (without options) to print a few lines or lines that I feel 
will have no special formatting.  Printf is quite useful when you want 
formatting and fast when printing several lines in one go.

> Last but not least, I'd say the argument parsing should be done a bit
> differently, but I'd have to have a closer look on exactly how it 
> should
> be done. The basic idea is: Parse global options first, then parse
> commands and have the function implementing a particular command parse
> its respective options.

Yes and no.

That's what I did before when I was trying to implement the new 
interface.  I found this to be both impractical and confusing, as it 
adds a third (optional) level to the command line order or attempt.

Consider that for the global option to work, the program needs a command 
as an argument, which is mandatory.  The global option would then 
fulfill its function based on a specific command.  So, in Qi, there are 
no global options as such (that is, general options that apply to all 
commands); instead, these are common options that can exist between one 
command and another, currently there are no options in Qi that apply to 
all commands.

Global options seem to be an illusion more than anything else, in 
practice you could type:

    qi -N install package.tlz

Note that -N in the first position (before the command) is a Global 
option.  But if you or someone forgets to type the -G option at this 
position, you'll want to match the option for the "install" command 
after it, so that it takes effect:

    qi install -N package.tlz

Not to mention that making the global options require a little more code 
in Qi to work, and this will be optional, since the user may not write 
the global option in that corresponding position.

It's clearer and simpler if you know what the order of Qi's interface 
is:

    qi COMMAND [OPTIONS] [FILE]...

> --Michael
>
> [1] See point 5. in
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_01

Thanks,
Matías