Re: [PATCH 2/2] parse-options: use and require int pointer for OPT_CMDMODE

From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Date: Sep 18, 2023, 10:10 UTC
Message-ID: <ZQgiD0ivfYRpSbnJ@ugly>
In-Reply-To: <6dc558c6-f78c-4d9c-8444-498de8e4d22a@web.de>

On Mon, Sep 18, 2023 at 11:28:31AM +0200, René Scharfe wrote:

Show 7 quoted lines

>@@ -2300,12 +2301,12 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar
> 				     "--show-current-patch", arg);
> 	}
>
>-	if (resume->mode == RESUME_SHOW_PATCH && new_value != resume->sub_mode)
>+	if (resume->mode_int == RESUME_SHOW_PATCH && new_value != resume->sub_mode)
>

this illustrates why i don't quite like the approach: the context determines which variable to use.

my idea would be to introduce a new type OPTION_SET_ENUM which would also use the callback field. one could even adjust the data type and elide the callback when c23 mode (or more specifically, the enum size feature) is detected.

> 		return error(_("options '%s=%s' and '%s=%s' "
> 					   "cannot be used together"),

> 					 "--show-current-patch", "--show-current-patch", arg, valid_modes[resume->sub_mode]);
>

totally on a tangent: the argument order is bogus here. and the line wrapping is also funny.

regards