From 3030197b8902ca65e1b3c3c58bdf4234c25f4da4 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 9 Jun 2022 22:22:24 +0100 Subject: [PATCH] Add error message for ambiguous long-form option abbreviation Before: $ set -o hist -ksh: set: hist: bad option(s) After: $ set --hist -ksh: set: hist: ambiguous option In ksh 93u+m, there are three options starting with 'hist', so the abbreviation does not represent a single option. It is useful to communicate this in the error message. In addition, "bad option(s)" was changed to "unknown option", the same message as that given by AST optget(3), for consistency. src/cmd/ksh93/sh/string.c: - Make sh_lookopt() return -1 for an ambiguous option. This is trivial as there is already an 'amb' variable flagging that up. src/cmd/ksh93/sh/args.c: - Use the negative sh_lookopt() return status to decide between "unknown option" and "ambiguous option". src/cmd/ksh93/data/builtins.c: sh_set[]: - Explain the --option and --nooption forms in addition to the -o option and +o option forms. - Document the long options without their 'no' prefixes (e.g. glob instead of noglob) as this simplifies documentation and the relation with the short options makes more sense. Those names are also how they show up in 'set -o' output and it is better for those to match. - Tweaks. --- NEWS | 3 +++ src/cmd/ksh93/bltins/test.c | 2 +- src/cmd/ksh93/data/builtins.c | 46 ++++++++++++++++++---------------- src/cmd/ksh93/data/msg.c | 1 - src/cmd/ksh93/data/options.c | 5 ++-- src/cmd/ksh93/include/argnod.h | 1 - src/cmd/ksh93/sh/args.c | 2 +- src/cmd/ksh93/sh/string.c | 8 +++++- src/cmd/ksh93/tests/basic.sh | 2 +- 9 files changed, 40 insertions(+), 30 deletions(-) diff --git a/NEWS b/NEWS index 7223a2e66..52d44e4d5 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,9 @@ Any uppercase BUG_* names are modernish shell bug IDs. - An interactive shell initialized in POSIX mode now once again has the preset aliases defined. This amends a change made on 2020-09-11. +- If a long-form shell option name is abbreviated so it matches multiple + options, the error message from ksh or 'set' now reads "ambiguous option". + 2022-06-08: - If -B/--braceexpand is turned on in --posix mode, it now only allows brace diff --git a/src/cmd/ksh93/bltins/test.c b/src/cmd/ksh93/bltins/test.c index fc6a52de9..064543617 100644 --- a/src/cmd/ksh93/bltins/test.c +++ b/src/cmd/ksh93/bltins/test.c @@ -452,7 +452,7 @@ int test_unop(register int op,register const char *arg) if(*arg=='?') return(sh_lookopt(arg+1,&f)>0); op = sh_lookopt(arg,&f); - return(op && (f==(sh_isoption(op)!=0))); + return(op>0 && (f==(sh_isoption(op)!=0))); case 't': { char *last; diff --git a/src/cmd/ksh93/data/builtins.c b/src/cmd/ksh93/data/builtins.c index 8f0883de2..d39b68002 100644 --- a/src/cmd/ksh93/data/builtins.c +++ b/src/cmd/ksh93/data/builtins.c @@ -195,9 +195,13 @@ const char sh_set[] = "their current settings to standard output. " "A \b+o\b with no \aoption\a writes a command that the shell can run " "to restore the current options state. " - "\b-o\b \aoption\a turns on \aoption\a and \b+o\b \aoption\a turns it " + "\b-o\b \aoption\a or \b--\b\aoption\a turns on \aoption\a " + "and \b+o\b \aoption\a or \b--no\b\aoption\a turns it " "off. This can be repeated to enable/disable multiple options. " - "The value of \aoption\a must be one of the following:]{" + "The value of \aoption\a is case-sensitive but insensitive to \b-\b " + "and \b_\b, and may be abbreviated to a non-arbitrary string. " + "It must resolve to one of the following:]" + "{" "[+allexport?Equivalent to \b-a\b.]" "[+backslashctrl?The backslash character \b\\\b escapes the " "next control character in the \bemacs\b built-in " @@ -207,20 +211,24 @@ const char sh_set[] = #if SHOPT_BRACEPAT "[+braceexpand?Equivalent to \b-B\b.] " #endif + "[+clobber?Opposite of \b-C\b.]" #if SHOPT_ESH "[+emacs?Enables/disables \bemacs\b editing mode.]" #endif "[+errexit?Equivalent to \b-e\b.]" + "[+exec?Opposite of \b-n\b.]" "[+functrace?Function scopes and subshells inherit the parent " - "environment's \bDEBUG\b trap action. Function scopes inherit " - "the \b-x\b option's state.]" + "environment's \bDEBUG\b trap action. Function scopes " + "inherit the \b-x\b option's state.]" + "[+glob?Opposite of \b-f\b.]" #if SHOPT_GLOBCASEDET "[+globcasedetect?Pathname expansion and file name completion " - "automatically become case-insensitive on file systems where " - "the difference between upper- and lowercase is ignored for " - "file names. Each slash-separated path name component pattern " - "\ap\a is treated as \b~(i:\b\ap\a\b)\b if its parent directory " - "exists on a case-insensitive file system.]" + "automatically become case-insensitive on file systems " + "where the difference between upper- and lowercase is " + "ignored for file names. Each slash-separated path name " + "component pattern \ap\a is treated as \b~(i:\b\ap\a\b)\b " + "if its parent directory exists on a case-insensitive " + "file system.]" #endif "[+globstar?Equivalent to \b-G\b.]" #if SHOPT_ESH @@ -231,11 +239,11 @@ const char sh_set[] = #if SHOPT_HISTEXPAND "[+histexpand?Equivalent to \b-H\b.]" #if SHOPT_ESH || SHOPT_VSH - "[+histreedit?If a history expansion (see \bhistexpand\b) " + "[+histreedit?If a history expansion (see \b-H\b) " "fails, the command line is reloaded into the next " "prompt's edit buffer, allowing corrections.]" "[+histverify?The results of a history expansion (see " - "\bhistexpand\b) are not immediately executed. " + "\b-H\b) are not immediately executed. " "Instead, the expanded line is loaded into the next " "prompt's edit buffer, allowing further changes.]" #endif @@ -252,12 +260,8 @@ const char sh_set[] = "[+multiline?Use multiple lines when editing lines that are " "longer than the window width.]" #endif - "[+noclobber?Equivalent to \b-C\b.]" - "[+noexec?Equivalent to \b-n\b.]" - "[+noglob?Equivalent to \b-f\b.]" - "[+nolog?Obsolete; has no effect.]" + "[+log?Obsolete; has no effect.]" "[+notify?Equivalent to \b-b\b.]" - "[+nounset?Equivalent to \b-u\b.]" "[+pipefail?A pipeline will not complete until all components " "of the pipeline have completed, and the exit status " "of the pipeline will be the value of the last " @@ -268,6 +272,7 @@ const char sh_set[] = "[+showme?Simple commands preceded by a \b;\b will be traced " "as if \b-x\b were enabled but not executed.]" "[+trackall?Equivalent to \b-h\b.]" + "[+unset?Opposite of \b-u\b.]" "[+verbose?Equivalent to \b-v\b.]" #if SHOPT_VSH "[+vi?Enables/disables \bvi\b editing mode.]" @@ -275,16 +280,13 @@ const char sh_set[] = "edit mode.]" #endif "[+xtrace?Equivalent to \b-x\b.]" -"}" + "}" /* * --posix is an AST optget(3) default option, so for ksh to use it, it must be listed * explicitly (and handled by sh_argopts() in sh/args.c) to stop optget(3) overriding it. - * Since it must appear here, use it as an example to document how --option == -o option. */ -"[05:posix?For any \b-o\b option (such as \bposix\b), \b--posix\b is equivalent " - "to \b-o posix\b and \b--noposix\b is equivalent to \b+o posix\b. " - "However, option names with a \bno\b prefix " - "are turned on by omitting \bno\b.]" +"[05:posix?Enable the \bposix\b option. When given at invocation time, " + "disables importing variable type attributes from the environment.]" "[p?Privileged mode. Disabling \b-p\b sets the effective user ID to the " "real user ID, and the effective group ID to the real group ID. " "Enabling \b-p\b restores the effective user and group IDs to their " diff --git a/src/cmd/ksh93/data/msg.c b/src/cmd/ksh93/data/msg.c index c19d0fcb0..3f6199948 100644 --- a/src/cmd/ksh93/data/msg.c +++ b/src/cmd/ksh93/data/msg.c @@ -49,7 +49,6 @@ const char e_mailmsg[] = "you have mail in $_"; const char e_query[] = "no query process"; const char e_history[] = "no history file"; const char e_histopen[] = "cannot open history file"; -const char e_option[] = "%s: bad option(s)"; const char e_optincompat1[] = "%s cannot be used with other options"; const char e_optincompat2[] = "%s cannot be used with %s"; const char e_toomany[] = "open file limit exceeded"; diff --git a/src/cmd/ksh93/data/options.c b/src/cmd/ksh93/data/options.c index 7f430c647..b948af87f 100644 --- a/src/cmd/ksh93/data/options.c +++ b/src/cmd/ksh93/data/options.c @@ -24,8 +24,9 @@ #include "shtable.h" /* - * This is the list of invocation and set options - * This list must be in ASCII sorted order + * This is the list of invocation and 'set' options. + * It must be sorted in ASCII order except for the no- prefix. + * See sh_lookopt() in string.c. */ const Shtable_t shtab_options[] = diff --git a/src/cmd/ksh93/include/argnod.h b/src/cmd/ksh93/include/argnod.h index b9863b481..5f2b6610d 100644 --- a/src/cmd/ksh93/include/argnod.h +++ b/src/cmd/ksh93/include/argnod.h @@ -134,7 +134,6 @@ extern int sh_argopts(int,char*[]); extern const char e_heading[]; extern const char e_subst[]; -extern const char e_option[]; extern const char e_exec[]; extern const char e_devfdNN[]; extern const char e_devfdstd[]; diff --git a/src/cmd/ksh93/sh/args.c b/src/cmd/ksh93/sh/args.c index 36701ec96..c12e0c472 100644 --- a/src/cmd/ksh93/sh/args.c +++ b/src/cmd/ksh93/sh/args.c @@ -156,7 +156,7 @@ int sh_argopts(int argc,register char *argv[]) o = sh_lookopt(opt_info.arg,&f); if(o<=0 || (setflag && (o&SH_COMMANDLINE))) { - errormsg(SH_DICT,2, e_option, opt_info.arg); + errormsg(SH_DICT,2, "%s: %s option", opt_info.arg, o<0 ? "ambiguous" : "unknown"); error_info.errors++; } o &= 0xff; diff --git a/src/cmd/ksh93/sh/string.c b/src/cmd/ksh93/sh/string.c index df82fb3b0..3b9c45f60 100644 --- a/src/cmd/ksh93/sh/string.c +++ b/src/cmd/ksh93/sh/string.c @@ -65,6 +65,12 @@ const Shtable_t *sh_locate(register const char *sp,const Shtable_t *table,int si /* * shtab_options lookup routine + * + * Long-form option names are case-sensitive but insensitive to '-' and '_', and may be abbreviated to a + * non-arbitrary string. A no- prefix is skipped and inverts the meaning (special handling for 'notify'). + * The table must be sorted in ASCII order after skipping the no- prefix. + * + * Returns 0 if not found, -1 if multiple found (ambiguous), or the number of the option found. */ #define sep(c) ((c)=='-'||(c)=='_') @@ -163,7 +169,7 @@ int sh_lookopt(register const char *sp, int *invert) } if(hit) *invert ^= inv; - return(hit); + return(amb ? -1 : hit); } /* diff --git a/src/cmd/ksh93/tests/basic.sh b/src/cmd/ksh93/tests/basic.sh index 4a52c6bc4..7fc2207cf 100755 --- a/src/cmd/ksh93/tests/basic.sh +++ b/src/cmd/ksh93/tests/basic.sh @@ -905,7 +905,7 @@ done # ksh2020 regression: https://github.com/att/ast/issues/1284 actual=$($SHELL --verson 2>&1) actual_status=$? -expect=': verson: bad option(s)' +expect=': verson: @(bad option(s)|unknown option)' expect_status=2 [[ "$actual" == *${expect}* ]] || err_exit "failed to handle invalid flag" \ "(expected *$(printf %q "$expect")*, got $(printf %q "$actual"))"