Re: [PATCH 2/2] format-patch: Add --cover-letter-wrap
From: Stephen Boyd <hidden>
Date: 2016-06-15 22:47:00
Joe Perches wrote:
quoted hunk ↗ jump to hunk
@@ -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?
quoted hunk ↗ jump to hunk
+ 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>]]).