Re: [PATCH v4 3/3] format-patch: teach --cover-from-description option
Denton Liu writes:
> +format.coverFromDescription::
> + The default mode for format-patch to determine which parts of
> + the cover letter will be populated using the branch's
> + description. See the `--cover-from-description` option in
> + linkgit:git-format-patch[1].
> +
> format.signature::
> The default for format-patch is to output a signature containing
> the Git version number. Use this variable to change that default.
> diff --git a/Documentation/git-format-patch.txt
> b/Documentation/git-format-patch.txt
> index 0ac56f4b70..86114e4c22 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -19,6 +19,7 @@ SYNOPSIS
> [--start-number ] [--numbered-files]
> [--in-reply-to=] [--suffix=.]
> [--ignore-if-in-upstream]
> +[--cover-from-description=]
> [--rfc] [--subject-prefix=]
> [(--reroll-count|-v) ]
> [--to=] [--cc=]
> @@ -171,6 +172,26 @@ will want to ensure that threading is disabled for `git
> send-email`.
> patches being generated, and any patch that matches is
> ignored.
>
> +--cover-from-description=::
> + Controls which parts of the cover letter will be automatically
> + populated using the branch's description.
> ++
> +If `` is `message` or `default`, the cover letter subject will be
> +populated with placeholder text. The body of the cover letter will be
> +populated with the branch's description.
I understand that this is what we do now, so those who want to live
in the past can set the configuration variable to 'message'.
> +If `` is `subject`, the beginning of the branch description (up to
> +the first blank line) will populate the cover letter subject. The
> +remainder of the description will populate the body of the cover
> +letter.
s/the beginning of .*blank line)/the first paragraph of the branch description/
may be shorter, but the above is OK, too.
When description is prepared appropriately, this mode would fill
both subject and body, which sounds sensible.
> +If `` is `auto`, if the beginning of the branch description (up to
> +the first line) is greater than 100 characters then the mode will be
> +`message`, otherwise `subject` will be used.
I understand that this is a more clever and safer variant of
'subject'. Do you want to say 100 characters or 100 bytes?
> +If `` is `none`, both the cover letter subject and body will be
> +populated with placeholder text.
OK, this is done for completeness? I wonder who finds it useful to
set it to 'none' *AND* set the branch description. Not a rhetorical
question that suggests removing this choice, but purely soliciting
opinions from others.
It is unclear (other than the mode word being 'default' for one of
the choices) what the new default mode of operation is after the
patch is applied among the four presented mode. "This is the
default when no configuration nor command line option specifies the
desired mode" or something may want to be added to one of these
paragraphs.
> @@ -1061,13 +1076,16 @@ static void make_cover_letter(struct rev_info *rev,
> int use_stdout,
> struct commit *origin,
> int nr, struct commit **list,
> const char *branch_name,
> + enum cover_from_description
> cover_from_description_mode,
> int quiet)
> {
> const char *committer;
> - const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n";
> - const char *msg;
> + const char *subject = "*** SUBJECT HERE ***";
> + const char *body = "*** BLURB HERE ***";
> struct shortlog log;
> struct strbuf sb = STRBUF_INIT;
> + struct strbuf description_sb = STRBUF_INIT;
> + struct strbuf subject_sb = STRBUF_INIT;
> int i;
> const char *encoding = "UTF-8";
> int need_8bit_cte = 0;
> @@ -1095,17 +1113,34 @@ static void make_cover_letter(struct rev_info *rev,
> int use_stdout,
> if (!branch_name)
> branch_name = find_branch_name(rev);
>
> - msg = body;
> + if (branch_name && *branch_name)
> + read_branch_desc(&description_sb, branch_name);
It may not matter in practice but strictly speaking there is no need
to read the description if we know that the mode is NONE. Removing
the support for the NONE mode may be an easier fix than adding "&&
mode != NONE" to the if () condition guarding this call---I dunno.
> + if (cover_from_description_mode != COVER_FROM_NONE &&
> description_sb.len) {
> + if (cover_from_description_mode == COVER_FROM_SUBJECT ||
> + cover_from_description_mode == COVER_FROM_AUTO)
> + body = format_subject(&subject_sb, description_sb.buf,
> " ");
> +
> + if (cover_from_description_mode == COVER_FROM_MESSAGE ||
> +
[PATCH v4 3/3] format-patch: teach --cover-from-description option
Before, when format-patch generated a cover letter, only the body would
be populated with a branch's description while the subject would be
populated with placeholder text. However, users may want to have the
subject of their cover letter automatically populated in the same way.
Teach format-patch to accept the `--cover-from-description` option and
corresponding `format.coverFromDescription` config, allowing users to
populate different parts of the cover letter (including the subject
now).
Signed-off-by: Denton Liu
---
Documentation/config/format.txt| 6 +
Documentation/git-format-patch.txt | 22
builtin/log.c | 84 ++
t/t4014-format-patch.sh| 172 +
4 files changed, 263 insertions(+), 21 deletions(-)
diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index cb629fa769..735dfcf827 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -36,6 +36,12 @@ format.subjectPrefix::
The default for format-patch is to output files with the '[PATCH]'
subject prefix. Use this variable to change that prefix.
+format.coverFromDescription::
+ The default mode for format-patch to determine which parts of
+ the cover letter will be populated using the branch's
+ description. See the `--cover-from-description` option in
+ linkgit:git-format-patch[1].
+
format.signature::
The default for format-patch is to output a signature containing
the Git version number. Use this variable to change that default.
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 0ac56f4b70..86114e4c22 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -19,6 +19,7 @@ SYNOPSIS
[--start-number ] [--numbered-files]
[--in-reply-to=] [--suffix=.]
[--ignore-if-in-upstream]
+ [--cover-from-description=]
[--rfc] [--subject-prefix=]
[(--reroll-count|-v) ]
[--to=] [--cc=]
@@ -171,6 +172,26 @@ will want to ensure that threading is disabled for `git
send-email`.
patches being generated, and any patch that matches is
ignored.
+--cover-from-description=::
+ Controls which parts of the cover letter will be automatically
+ populated using the branch's description.
++
+If `` is `message` or `default`, the cover letter subject will be
+populated with placeholder text. The body of the cover letter will be
+populated with the branch's description.
++
+If `` is `subject`, the beginning of the branch description (up to
+the first blank line) will populate the cover letter subject. The
+remainder of the description will populate the body of the cover
+letter.
++
+If `` is `auto`, if the beginning of the branch description (up to
+the first line) is greater than 100 characters then the mode will be
+`message`, otherwise `subject` will be used.
++
+If `` is `none`, both the cover letter subject and body will be
+populated with placeholder text.
+
--subject-prefix=::
Instead of the standard '[PATCH]' prefix in the subject
line, instead use '[]'. This
@@ -347,6 +368,7 @@ with configuration variables.
signOff = true
outputDirectory =
coverLetter = auto
+ inferCoverSubject = true
diff --git a/builtin/log.c b/builtin/log.c
index f06f5d586b..0cc8b59991 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -37,6 +37,7 @@
#include "range-diff.h"
#define MAIL_DEFAULT_WRAP 72
+#define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100
/* Set a default date-time format for git log ("log.date" config variable) */
static const char *default_date_mode = NULL;
@@ -777,6 +778,13 @@ enum thread_level {
THREAD_DEEP
};
+enum cover_from_description {
+ COVER_FROM_NONE,
+ COVER_FROM_MESSAGE,
+ COVER_FROM_SUBJECT,
+ COVER_FROM_AUTO
+};
+
static enum thread_level thread;
static int do_signoff;
static int base_auto;
@@ -785,6 +793,23 @@ static const char *signature = git_version_string;
static const char *signature_file;
static enum cover_setting config_cover_letter;
static const char *config_output_directory;
+static enum cover_from_description cover_from_description_mode =
COVER_FROM_MESSAGE;
+
+static enum cover_from_description parse_cover_from_description(const char
*arg)
+{
+ if (!arg || !strcmp(arg, "default"))
+ return COVER_FROM_MESSAGE;
+ else if (!strcmp(arg, "none"))
+ return COVER_FROM_NONE;
+ else if (!strcmp(arg, "message"))
+ return COVER_FROM_MESSAGE;
+ else if (!strcmp(arg, "subject"))
+ return COVER_FROM_SUBJECT;
+ else if (!strcmp(arg, "auto"))
+ return COVER_FROM_AUTO;
+ else
+ die(_("%s: invalid cover from description mode")
