sh: accept fc options grouped behind one '-'

[pstef]

Summary

As per Utility Syntax Guidelines, accept both forms: -l -n and -ln.

To do that, anticipate the source string for the next option that will
be parsed by nextopt(). It's not always *argptr, sometimes it is
nextopt_optptr.

To simplify the check for not_fcnumber, slightly modify nextopt() to
always nullify nextopt_optptr in cases where it would have been set
to point to a NUL character.

[pstef]

I've made the mistake of assuming that the first token has to be an option, but of course fc -1 must work.

Slightly change the strategy to fix the original issue and keep the not_fcnumber() check first as it was originally.

[pstef]

@jilles what do you think about this one?

[jilles]

This should have testcases.

bin/sh/histedit.c

279

If nextopt_optptr is not equal to *argptr and is an fcnumber, it will still attempt to use *argptr below. This would affect cases like fc -n1.

[pstef]

Slightly change the approach: each not_fcnumber() check, keep consuming options until we need to jump to the next argptr.
Add a couple of test cases.

[jilles]

bin/sh/histedit.c

281

Nit (pre-existing issue): this (char) cast is unnecessary, and other nextopt callers don't use it.

bin/sh/tests/builtins/fc3.0

4

Please avoid the == in [ non-standard feature (but see below).

10–11

I tend to make test scripts portable (apart from the thing being tested for, if it is not standard), so that they can run with a variety of shells. Also, if the only thing that prevents running the test successfully in shells/heirloom-sh is $(...) command substitution, I tend to replace it with the traditional form.

Here, break and continue outside of a loop is not standard, and in various shells such as bash, mksh and zsh it writes an error message (and might also abort a script; I haven't checked that). Instead, : command1 and : command2 could be used.

bin/sh/tests/functional_test.sh

37–39 ↗

(On Diff #108741)

Hmm, fc1.0 and fc2.0 also fail if history support is not compiled in. I would be fine with not having this check.

[pstef]

@jilles is this patch in a good shape now?

Or maybe we should convert sh to use getopt? Custom options parsing keeps surprising people, see PR265399 for a recent example.

[jilles]

In D35836#823357, @pstef wrote:

@jilles is this patch in a good shape now?

Yes.

Or maybe we should convert sh to use getopt? Custom options parsing keeps surprising people, see PR265399 for a recent example.

The echo builtin is defined not to support standard options parsing, and changing that will lead to compatibility issues. In the particular case of echo, I don't think it's worth it since there is no obviously best solution so there will still be many bad surprises after a fix.

As for builtins that support standard options parsing, some of them used to use getopt() from libc, but we standardized on nextopt() to reduce code duplication and code size. For example, the error messages from getopt() cannot be used because they are sent to stderr instead of a potentially redirected out2 from output.c, for example echo ".$(umask -\? 2>&1)." .