‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Saturday, August 28, 2021 4:49 PM, Robert Elz <k...@munnari.oz.au> wrote:
> Date: Sat, 28 Aug 2021 15:26:28 +0000 > From: hancooper <hancoo...@protonmail.com> > > Message-ID: > <TOCayWuYa29hFEyrsgu0Xioa0wQvYZ3z3Z1-YtwvinA6y_Xglwl5p-_oedj3Vy5EZQMZqfbipHIUp8Ll6fWNw9tor_JiZvSAQVG0kXEuCFM=@protonmail.com> > > > | Would the code break if I use shortopts="Vuhv:s" (allows getopts > | to issue errors, not in silent mode) but also have the (":") and > | ("?") checks inside the case statement? > > Not break, but the ':' case will be a waste of space, as that cannot > happen in that case, the ':' return only happens with the leading ':' > in the opts string (I didn't bother to quote that part of the spec last > time, as it wasn't immediately relevant). The reason for the code is that it would be easy to use either Vuhv:s or :Vuhv:s without having to modify the case statements when using Vuhv:s. > | As so > > That should work, the ":" case (as above) won't ever happen. > > There's also a whole lot of meaningless quoting that you could delete > (shell doesn't have strings ... or perhaps that is better stated as > everything is a string, quotes aren't needed to make something into one - > quotes are only needed where some shell magic character appears (to literally > match the '?' that needs to be quoted) or to protect the contents of > an expansion from further processing. > > So: > > | local shortopts="Vuhvs" > > quotes there are useless. > > | while getopts $shortopts arg; do > > but (double-)quoting around $shortopts can be a good idea, > as anything that results from an expansion is subject to > pathname expansion and field splitting ... that's not going to > be an issue here, as the value of shortopts is known, and > contains nothing which would trigger pathname expansion, and > is unlikely to result in field splitting either (depends on > the value of IFS, so it could happen) - but always quoting > things like that is a good idea. Greg's FAQ would tell you > it is essential ... I don't go quite that far (just 95% of the way). > > | case $arg in > > but quotes aren't essential there, because the word in a case statement > isn't subject to those extra expansions (it wouldn't hurt to (double-)quote > it though, for consistency) > > | ("V") > > Those quotes are useless (all the useless quotes here are also harmless, > so if you really like the look of them, they do no harm - but they're > unconventional in sh code). > > | printf '%s\n' "Version" > > The \ needs quoting, one way or another, so the single quotes > are reasonable ( %s\\n would be another way ) but the double > quotes around Version are meaningless. [Aside: if you are going > to add meaningless quoting, using single quotes is generally > better, the shell doesn't need to parse the insides of a single > quoted word nearly as much as a double quoted one, so processing > single quoted strings is marginally faster - but probably so marginally > that you'd never be able to measure the difference]. > > | ("v") > | vb="$OPTARG" > > That's another place (the assignment) where the quotes aren't really > needed, as neither pathname expansion nor field splitting occur there, > but again, for consistency, keeping them can be a good idea. > > | ("?") > | printf '%s\n' "Option not recognised by shortopts" > | printf '%s\n'pfm "Invalid option: -${OPTARG}." > | break > | ;; > > You don't need the error messages, getopts will have issued > one already, the point of the '?' case in this scenario is > to allow the code to clean up (which the "break" is intended > to achieve I suspect, but that's rarely the right choice, as > it just falls out of the while loop and continues executing > the script ... exit is a more common thing to do there. > > | (*) > | printf '%s\n' "Invoke \`getopts_test -h' for details." > | ;; > > That one is missing a break (or more likely, exit) which is likely > needed for the same reason as the previous one. But since the var > set by getopts only ever returns one char, unquoting the ? in the > previous pattern would allow these two cases to be combined, the '?' > (if unquoted) would match anything not previously matched, both > a literal '?' for the error case, but also some other option where > the char was added to the opts string (shortopts here) but the > case pattern to handle it was forgotten. > > kre