Re: [PATCH 2/4] format-rev: factor option variables into a struct

[email protected] writes:

> From: Kristoffer Haugsbakk <[email protected]>
>
> We will in two commits add three more options to this command.
> Let’s prepare for that by moving option variables into a struct
> so that we get less local variables.
>
> This allows us to inline `format_nul_data` into this new
> structure. Let’s also rename `stdin_mode_arg` to `stdin_mode`.
> (We couldn’t use `stdin_mode` before because of the enumeration
> with the same name.)
>
> Signed-off-by: Kristoffer Haugsbakk <[email protected]>
> ---
>  builtin/name-rev.c | 44 +++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index 254c88199fd..7d824aa1c5d 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -772,16 +772,19 @@ int cmd_name_rev(int argc,
>  	return 0;
>  }
>  
> -struct format_nul_data {
> +struct format_rev_data {
> +	const char *format;
> +	const char *stdin_mode;
>  	bool nul_input;
>  	bool nul_output;
> +	struct string_list notes;
>  };
>  
>  static int format_nul_cb(const struct option *option,
>  			 const char *arg,
>  			 int unset)
>  {
> -	struct format_nul_data *data = option->value;
> +	struct format_rev_data *data = option->value;
>  	data->nul_input = 1;
>  	data->nul_output = 1;
>  	BUG_ON_OPT_NEG(unset);
> @@ -813,31 +816,30 @@ int cmd_format_rev(int argc,
>  		   const char *prefix,
>  		   struct repository *repo UNUSED)
>  {
> -	const char *format = NULL;
> +	struct format_rev_data data = {
> +		NULL, NULL, 0, 0, STRING_LIST_INIT_NODUP
> +	};

It will make it easier to maintain if you used designated
initializer here, i.e.,

	struct format_rev_data data = {
		.notes = STRING_LIST_INIT_NODUP,
	};

The other members not explicitly mentioned by the initializer will
be zero-initialized.

Other parts of the patch look good.

Thanks.