From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Date: Mon, 18 Sep 2023 10:10:23 GMT
Subject: Re: [PATCH 2/2] parse-options: use and require int pointer for OPT_CMDMODE
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:
>@@ -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