Re: [PATCH 2/2] format-patch: Add --cover-letter-wrap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Joe Perches wrote:
> @@ -792,6 +806,27 @@ static int output_directory_callback(const struct option *opt, const char *arg,
>  	return 0;
>  }
>  
> +static int cls_callback(const struct option *opt, const char *arg, int unset)
> +{
> +	if (unset)
> +		cls.cover_letter_wrap = 0;
> +	else {
> +		int i1, i2, i3;
> +		if (!arg)
> +			return 1;
> +		int arg_count = sscanf(arg, "%d,%d,%d", &i1, &i2, &i3);
> +		if (arg_count <= 0)
> +			return 1;
> +		if (arg_count >= 1)
> +			cls.cover_letter_wrappos = i1;
> +		if (arg_count >= 2)
> +			cls.cover_letter_indent1 = i2;
> +		if (arg_count >= 3)
> +			cls.cover_letter_indent2 = i3;
> +		}

This bracket is one indent off.

I'm not sure, but can this be simplified to just setting the struct
members directly through sscanf? You won't need to have these if's in
that case. I think something like --cover-letter-wrap="" would be
equivalent to just using the defaults and not an error. Does that sound
right?

> +	return 0;
> +}
> +
>  static int thread_callback(const struct option *opt, const char *arg, int unset)
>  {
>  	int *thread = (int *)opt->value;
> @@ -875,6 +910,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  			    "print patches to standard out"),
>  		OPT_BOOLEAN(0, "cover-letter", &cover_letter,
>  			    "generate a cover letter"),
> +		{ OPTION_CALLBACK, 0, "cover-letter-wrap", &cls, NULL,
> +			    "control the cover letter format",
> +			    PARSE_OPT_OPTARG, cls_callback },

Why is this PARSE_OPT_OPTARG? I only see the choice of having arguments
or prefixed with a --no. Also, please use PARSE_OPT_LITERAL_ARGHELP and
give it the help string you use in the docs
(<width>[,<indent1>[,<indent2>]]).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]