Daniel Barkalow <barkalow@xxxxxxxxxxxx> writes:
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 651efe6..7ec01a0 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -17,6 +17,7 @@ SYNOPSIS
> [--in-reply-to=Message-Id] [--suffix=.<sfx>]
> [--ignore-if-in-upstream]
> [--subject-prefix=Subject-Prefix]
> + [--cover-letter]
> [ <since> | <revision range> ]
>
> DESCRIPTION
> @@ -139,6 +140,11 @@ include::diff-options.txt[]
> Instead of using `.patch` as the suffix for generated
> filenames, use specified suffix. A common alternative is
> `--suffix=.txt`.
> +
> +--cover-letter::
> + Generate a cover letter template. You still have to fill in
> + a description, but the shortlog and the diffstat will be
> + generated for you.
> +
> Note that you would need to include the leading dot `.` if you
> want a filename like `0001-description-of-my-change.patch`, and
Huh? Leading dot? The insertion is one paragraph too low or high.
> diff --git a/builtin-log.c b/builtin-log.c
> index 1f74d66..c6e3f91 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -453,74 +454,77 @@ static int git_format_config(...
> ...
> static FILE *realstdout = NULL;
> static const char *output_directory = NULL;
>
> +static int reopen_stdout(const char *oneline, int nr, int total)
> {
> char filename[PATH_MAX];
> int len = 0;
> int suffix_len = strlen(fmt_patch_suffix) + 1;
>
> if (output_directory) {
> + len = snprintf(filename, sizeof(filename), "%s",
> + output_directory);
> + if (len >=
> sizeof(filename) - FORMAT_PATCH_NAME_MAX - suffix_len)
> return error("name of output directory is too long");
> if (filename[len - 1] != '/')
> filename[len++] = '/';
> }
>
> + if (!filename)
> + len += sprintf(filename + len, "%d", nr);
How can this trigger? Do you mean "oneline"?
I can understand that you do not want to pass numbered_files
variable separately to the function, but then there should be a
comment at the beginning of this function that says "oneline is
NULL under numbered_files mode, the caller is responsible for
ensuring that".
> + else {
> + len += sprintf(filename + len, "%04d-", nr);
> + len += snprintf(filename + len, sizeof(filename) - len - 1
> + - suffix_len, "%s", oneline);
> strcpy(filename + len, fmt_patch_suffix);
> }
>
> @@ -591,6 +595,74 @@ static void gen_message_id(struct rev_info *info, char *base)
> + /*
> + * We can only do diffstat with a unique reference point, and
> + * log is a bit tricky, so just skip it.
> + */
> + if (!origin)
> + return;
Maybe we would want to leave a note in the output to tell your
users that we punted here.
> + argv[0] = "shortlog";
> + argv[1] = head_sha1;
> + argv[2] = "--not";
> + argv[3] = origin_sha1;
> + argv[4] = NULL;
> + fflush(stdout);
> + run_command_v_opt(argv, RUN_GIT_CMD);
> +
> + argv[0] = "diff";
> + argv[1] = "--stat";
> + argv[2] = "--summary";
> + argv[3] = head_sha1;
> + argv[4] = "--not";
> + argv[5] = origin_sha1;
> + argv[6] = NULL;
> + fflush(stdout);
> + run_command_v_opt(argv, RUN_GIT_CMD);
Makes me wonder if we already have enough infrastructure to do
this without spawning two new processes. Not complaining, but
just wondering.
> @@ -776,6 +852,20 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> * get_revision() to do the usual traversal.
> */
> }
> + if (cover_letter) {
> + /* remember the range */
> + int i;
> + for (i = 0; i < rev.pending.nr; i++) {
> + struct object *o = rev.pending.objects[i].item;
> + if (o->flags & UNINTERESTING)
> + origin = (struct commit *)o;
> + else
> + head = (struct commit *)o;
> + }
What if you have more than one origin or head? Perhaps the
punting logic should be modified to make sure we only have one
negative and one positive and nothing else?
> + /* We can't generate a cover letter without any patches */
> + if (!head)
> + return 0;
> + }
That's true, but with or without cover letter we won't have
anything to format if we do not have any positive commit.
> @@ -802,9 +892,18 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
> numbered = 1;
> if (numbered)
> rev.total = total + start_number - 1;
> - rev.add_signoff = add_signoff;
> if (in_reply_to)
> rev.ref_message_id = clean_message_id(in_reply_to);
> + if (cover_letter) {
> + if (thread) {
> + gen_message_id(&rev, "cover");
> + }
> + make_cover_letter(&rev, use_stdout, numbered, numbered_files,
> + origin, head);
> + total++;
> + start_number--;
> + }
> + rev.add_signoff = add_signoff;
Nice attention to the detail, moving add_signoff after the cover
letter.
-
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
