git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 02/10 v2] parse-options: clearer reporting of API misuse

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Dec 2, 2010, 06:01 UTC
Message-ID
<20101202060118.GB32125@burratino>
In-Reply-To
<20101202045707.GA32125@burratino>

The PARSE_OPT_LASTARG_DEFAULT flag is meant for options like --contains that (1) traditionally had a mandatory argument and (2) have some better behavior to use when appearing in the final position. It makes no sense to combine this with OPTARG, so ever since v1.6.4-rc0~71 (parse-options: add parse_options_check to validate option specs, 2009-07-09) this mistake is flagged with

	error: `--option` uses incompatible flags LASTARG_DEFAULT and OPTARG
and an exit status representing an error in commandline usage.

Unfortunately that which might confuse scripters calling such an erroneous program into thinking the _script_ contains an error. Clarify that it is an internal error by dying with a message beginning "error: BUG: ..." and status 128.

While at it, clean up parse_options_check to prepare for more checks.

Long term, it would be nicer to make such checks happen at compile time.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:
> Jonathan Nieder wrote:
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -316,24 +323,12 @@ static void check_typos(const char *arg, const struct option *options)
[...]
Show 10 quoted lines
>> +		    (opts->flags & PARSE_OPT_OPTARG))
>> +			optbug(opts, "uses incompatible flags "
>> +				"LASTARG_DEFAULT and OPTARG");
>>  	}
>> -
>> -	if (err)
>> -		exit(129);
>
> Hmph, this is simpler but it does not report all errors any more.
> So it would be better to do:
Like this.  (Looks okay now.  Famous last words...)
 parse-options.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 196ba71..97d7ff7 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -11,6 +11,13 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx,
 #define OPT_SHORT 1
 #define OPT_UNSET 2
 
+static int optbug(const struct option *opt, const char *reason)
+{
+	if (opt->long_name)
+		return error("BUG: option '%s' %s", opt->long_name, reason);
+	return error("BUG: switch '%c' %s", opt->short_name, reason);
+}
+
 static int opterror(const struct option *opt, const char *reason, int flags)
 {
 	if (flags & OPT_SHORT)
@@ -320,20 +327,12 @@ static void parse_options_check(const struct option *opts)
 
 	for (; opts->type != OPTION_END; opts++) {
 		if ((opts->flags & PARSE_OPT_LASTARG_DEFAULT) &&
-		    (opts->flags & PARSE_OPT_OPTARG)) {
-			if (opts->long_name) {
-				error("`--%s` uses incompatible flags "
-				      "LASTARG_DEFAULT and OPTARG", opts->long_name);
-			} else {
-				error("`-%c` uses incompatible flags "
-				      "LASTARG_DEFAULT and OPTARG", opts->short_name);
-			}
-			err |= 1;
-		}
+		    (opts->flags & PARSE_OPT_OPTARG))
+			err |= optbug(opts, "uses incompatible flags "
+					"LASTARG_DEFAULT and OPTARG");
 	}
-
 	if (err)
-		exit(129);
+		exit(128);
 }
 
 void parse_options_start(struct parse_opt_ctx_t *ctx,
-- 
1.7.2.3
Previous: Jonathan NiederNext: Jonathan Nieder
Message 7 of 21 in “update-index: migrate to parse-options API”
  1. 00/10 update-index: migrate to parse-options APIJonathan Nieder, Dec 1, 2010
  2. 01/10 parse-options: Don't call parse_options_check() so muchJonathan Nieder, Dec 1, 2010
  3. René ScharfeDec 5, 2010
  4. Stephen BoydDec 6, 2010
  5. 02/10 parse-options: clearer reporting of API misuseJonathan Nieder, Dec 1, 2010
  6. Jonathan NiederDec 2, 2010
  7. 02/10 parse-options: clearer reporting of API misuseJonathan Nieder, Dec 2, 2010
  8. 02/10 parse-options: clearer reporting of API misuseJonathan Nieder, Dec 2, 2010
  9. 03/10 parse-options: move NODASH sanity checks to parse_options_checkJonathan Nieder, Dec 1, 2010
  10. 03/10 parse-options: move NODASH sanity checks to parse_options_checkJonathan Nieder, Dec 2, 2010
  11. 04/10 parse-options: sanity check PARSE_OPT_NOARG flagJonathan Nieder, Dec 1, 2010
  12. 04/10 parse-options: sanity check PARSE_OPT_NOARG flagJonathan Nieder, Dec 2, 2010
  13. 05/10 parse-options: do not infer PARSE_OPT_NOARG from option typeJonathan Nieder, Dec 1, 2010
  14. 06/10 parse-options: never suppress arghelp if LITERAL_ARGHELP is setJonathan Nieder, Dec 1, 2010
  15. Stephen BoydDec 3, 2010
  16. Jonathan NiederDec 3, 2010
  17. Stephen BoydDec 3, 2010
  18. 07/10 parse-options: allow git commands to invent new option typesJonathan Nieder, Dec 1, 2010
  19. 08/10 parse-options: make resuming easier after PARSE_OPT_STOP_AT_NON_OPTIONJonathan Nieder, Dec 1, 2010
  20. 09/10 setup: save prefix (original cwd relative to toplevel) in startup_infoJonathan Nieder, Dec 1, 2010
  21. 10/10 update-index: migrate to parse-options APIJonathan Nieder, Dec 1, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.