Re: [Dragora-members] Qi 2.0rc10

[Michael Siegel]

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.

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

  * 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.

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

  * L 1506: \unalias -a;                # Unset all possible aliases.
    * Why is there a backslash before "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.

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.

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

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.


--Michael

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