git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Johannes Sixt" <j6t@kdbg.org>,
	"Philip Oakley" <philipoakley@iee.email>
Subject: [PATCH v5 0/3] format-patch: learn --cover-from-description option
Date: Mon, 14 Oct 2019 13:46:55 -0700	[thread overview]
Message-ID: <cover.1571085952.git.liu.denton@gmail.com> (raw)
In-Reply-To: <cover.1570821015.git.liu.denton@gmail.com>

Currently, format-patch only puts "*** SUBJECT HERE ***" when a cover
letter is generated. However, it is already smart enough to be able to
populate the cover letter with the branch description so there's no
reason why it cannot populate the subject as well.

Teach format-patch the `--cover-from-description` option and
corresponding `format.coverFromDescription` configuration option which
will allow it to populate not only the body but the subject as well.

Changes since v4:

* Modify 1/3 to more closely reflect intent of the original author

* Incorporate Junio's suggestions into the documentation

* Extract branch desc logic into pp_from_desc()

* Fix broken tests

Changes since v3:

* Change --infer-cover-subject to --cover-from-description

* No more test cleanup patches (they were merged in
  'dl/format-patch-doc-test-cleanup')

Changes since v2:

* Break 1/4 into many different patches (one per paragraph of the
  original patch)

* Incorporate Eric's documentation/commit message suggestions

Changes since v1:

* Incorporate Eric's suggestions for cleanup in all patches

* Add patch 3/4 to make it clear what is the default value for
  format.coverLetter (since format.inferCoverSubject was borrowed from
  this config but it also did not state what the default value was)

* In 1/4, rename all instances of "expected" to "expect"

Denton Liu (3):
  format-patch: change erroneous and condition
  format-patch: use enum variables
  format-patch: teach --cover-from-description option

 Documentation/config/format.txt    |   6 +
 Documentation/git-format-patch.txt |  22 ++++
 builtin/log.c                      | 125 +++++++++++++++------
 t/t4014-format-patch.sh            | 172 +++++++++++++++++++++++++++++
 t/t9902-completion.sh              |   5 +-
 5 files changed, 296 insertions(+), 34 deletions(-)

Range-diff against v4:
1:  267bc00dc8 ! 1:  56fb230ad2 format-patch: remove erroneous and condition
    @@ Metadata
     Author: Denton Liu <liu.denton@gmail.com>
     
      ## Commit message ##
    -    format-patch: remove erroneous and condition
    +    format-patch: change erroneous and condition
     
         Commit 30984ed2e9 (format-patch: support deep threading, 2009-02-19),
         introduced the following lines:
    @@ Commit message
                 thread = git_config_bool(var, value) && THREAD_SHALLOW;
     
         Since git_config_bool() returns a bool, the trailing `&& THREAD_SHALLOW`
    -    is a no-op. Remove this erroneous and condition.
    +    is a no-op.
    +
    +    In Python, `x and y` is equivalent to `y if x else x`[1]. Since this
    +    seems to be a Python-ism that's mistakenly leaked into our code, convert
    +    this to the equivalent C expression.
    +
    +    [1]: https://docs.python.org/3/reference/expressions.html#boolean-operations
     
         Signed-off-by: Denton Liu <liu.denton@gmail.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## builtin/log.c ##
     @@ builtin/log.c: static int git_format_config(const char *var, const char *value, void *cb)
    @@ builtin/log.c: static int git_format_config(const char *var, const char *value,
      			return 0;
      		}
     -		thread = git_config_bool(var, value) && THREAD_SHALLOW;
    -+		thread = git_config_bool(var, value);
    ++		thread = git_config_bool(var, value) ? THREAD_SHALLOW : THREAD_UNSET;
      		return 0;
      	}
      	if (!strcmp(var, "format.signoff")) {
2:  638a5b40d2 ! 2:  e2769092fa format-patch: use enum variables
    @@ Commit message
         variables to use these new definitions.
     
         Signed-off-by: Denton Liu <liu.denton@gmail.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## builtin/log.c ##
     @@ builtin/log.c: static void add_header(const char *value)
3:  3289ce62bb ! 3:  315c308950 format-patch: teach --cover-from-description option
    @@ Commit message
         populate different parts of the cover letter (including the subject
         now).
     
    -    Signed-off-by: Denton Liu <liu.denton@gmail.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
    -
      ## Documentation/config/format.txt ##
     @@ Documentation/config/format.txt: format.subjectPrefix::
      	The default for format-patch is to output files with the '[PATCH]'
    @@ Documentation/git-format-patch.txt: will want to ensure that threading is disabl
     ++
     +If `<mode>` 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.
    ++populated with the branch's description. This is the default mode when
    ++no configuration nor command line option is specified.
     ++
    -+If `<mode>` 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 `<mode>` is `subject`, the first paragraph of the branch description will
    ++populate the cover letter subject. The remainder of the description will
    ++populate the body of the cover letter.
     ++
    -+If `<mode>` 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 `<mode>` is `auto`, if the first paragraph of the branch description
    ++is greater than 100 bytes, then the mode will be `message`, otherwise
    ++`subject` will be used.
     ++
     +If `<mode>` is `none`, both the cover letter subject and body will be
     +populated with placeholder text.
    @@ builtin/log.c: static void print_signature(FILE *file)
      static char *find_branch_name(struct rev_info *rev)
      {
      	int i, positive = -1;
    -@@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_stdout,
    +@@ builtin/log.c: static void show_diffstat(struct rev_info *rev,
    + 	fprintf(rev->diffopt.file, "\n");
    + }
    + 
    ++static void pp_from_desc(struct pretty_print_context *pp,
    ++			 const char *branch_name,
    ++			 struct strbuf *sb,
    ++			 const char *encoding,
    ++			 int need_8bit_cte)
    ++{
    ++	const char *subject = "*** SUBJECT HERE ***";
    ++	const char *body = "*** BLURB HERE ***";
    ++	struct strbuf description_sb = STRBUF_INIT;
    ++	struct strbuf subject_sb = STRBUF_INIT;
    ++
    ++	if (cover_from_description_mode == COVER_FROM_NONE)
    ++		goto do_pp;
    ++
    ++	if (branch_name && *branch_name)
    ++		read_branch_desc(&description_sb, branch_name);
    ++	if (!description_sb.len)
    ++		goto do_pp;
    ++
    ++	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 ||
    ++			(cover_from_description_mode == COVER_FROM_AUTO &&
    ++			 subject_sb.len > COVER_FROM_AUTO_MAX_SUBJECT_LEN))
    ++		body = description_sb.buf;
    ++	else
    ++		subject = subject_sb.buf;
    ++
    ++do_pp:
    ++	pp_title_line(pp, &subject, sb, encoding, need_8bit_cte);
    ++	pp_remainder(pp, &body, sb, 0);
    ++
    ++	strbuf_release(&description_sb);
    ++	strbuf_release(&subject_sb);
    ++}
    ++
    + 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,
    +@@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_stdout,
      			      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;
     @@ builtin/log.c: 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);
    -+
    -+	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 ||
    -+				(cover_from_description_mode == COVER_FROM_AUTO &&
    -+				 subject_sb.len > COVER_FROM_AUTO_MAX_SUBJECT_LEN))
    -+			body = description_sb.buf;
    -+		else
    -+			subject = subject_sb.buf;
    -+	}
    -+
      	pp.fmt = CMIT_FMT_EMAIL;
      	pp.date_mode.type = DATE_RFC2822;
      	pp.rev = rev;
    @@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_stdou
     -	pp_title_line(&pp, &msg, &sb, encoding, need_8bit_cte);
     -	pp_remainder(&pp, &msg, &sb, 0);
     -	add_branch_description(&sb, branch_name);
    -+	pp_title_line(&pp, &subject, &sb, encoding, need_8bit_cte);
    -+	pp_remainder(&pp, &body, &sb, 0);
    -+	strbuf_addch(&sb, '\n');
    ++	pp_from_desc(&pp, branch_name, &sb, encoding, need_8bit_cte);
      	fprintf(rev->diffopt.file, "%s\n", sb.buf);
      
    -+	strbuf_release(&description_sb);
    -+	strbuf_release(&subject_sb);
      	strbuf_release(&sb);
    - 
    - 	shortlog_init(&log);
     @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)
      	int use_patch_format = 0;
      	int quiet = 0;
    @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
      	if (0 < reroll_count) {
      		struct strbuf sprefix = STRBUF_INIT;
      		strbuf_addf(&sprefix, "%s v%d",
    -@@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)
    - 		if (thread)
    - 			gen_message_id(&rev, "cover");
    - 		make_cover_letter(&rev, use_stdout,
    --				  origin, nr, list, branch_name, quiet);
    -+				  origin, nr, list, branch_name, cover_from_description_mode, quiet);
    - 		print_bases(&bases, rev.diffopt.file);
    - 		print_signature(rev.diffopt.file);
    - 		total++;
     
      ## t/t4014-format-patch.sh ##
     @@ t/t4014-format-patch.sh: test_expect_success 'format patch ignores color.ui' '
    @@ t/t4014-format-patch.sh: test_expect_success 'format patch ignores color.ui' '
      test_expect_success 'cover letter using branch description (1)' '
      	git checkout rebuild-1 &&
      	test_config branch.rebuild-1.description hello &&
    +
    + ## t/t9902-completion.sh ##
    +@@ t/t9902-completion.sh: test_expect_success 'complete tree filename with metacharacters' '
    + '
    + 
    + test_expect_success PERL 'send-email' '
    +-	test_completion "git send-email --cov" "--cover-letter " &&
    ++	test_completion "git send-email --cov" <<-\EOF &&
    ++	--cover-from-description=Z
    ++	--cover-letter Z
    ++	EOF
    + 	test_completion "git send-email ma" "master "
    + '
    + 
-- 
2.23.0.17.g315c308950


  parent reply	other threads:[~2019-10-14 20:47 UTC|newest]

Thread overview: 91+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19 23:52 [PATCH v2 0/4] format-patch: learn --infer-cover-subject option Denton Liu
2019-08-19 23:52 ` [PATCH v2 1/4] t4014: clean up style Denton Liu
2019-08-20  2:41   ` Eric Sunshine
2019-08-19 23:52 ` [PATCH v2 2/4] Doc: add more detail for git-format-patch Denton Liu
2019-08-20  2:44   ` Eric Sunshine
2019-08-20  7:07     ` Denton Liu
2019-08-19 23:52 ` [PATCH v2 3/4] config/format.txt: make clear the default value of format.coverLetter Denton Liu
2019-08-20  2:47   ` Eric Sunshine
2019-08-19 23:52 ` [PATCH v2 4/4] format-patch: learn --infer-cover-letter option Denton Liu
2019-08-20  3:46   ` Eric Sunshine
2019-08-20  7:18 ` [PATCH v3 00/13] format-patch: learn --infer-cover-subject option (also t4014 cleanup) Denton Liu
2019-08-20  7:18   ` [PATCH v3 01/13] t4014: drop unnecessary blank lines from test cases Denton Liu
2019-08-20  7:18   ` [PATCH v3 02/13] t4014: s/expected/expect/ Denton Liu
2019-08-20 21:31     ` Eric Sunshine
2019-08-20  7:18   ` [PATCH v3 03/13] t4014: move closing sq onto its own line Denton Liu
2019-08-20  7:18   ` [PATCH v3 04/13] t4014: use sq for test case names Denton Liu
2019-08-20  7:18   ` [PATCH v3 05/13] t4014: remove spaces after redirect operators Denton Liu
2019-08-20  7:18   ` [PATCH v3 06/13] t4014: use indentable here-docs Denton Liu
2019-08-20  7:19   ` [PATCH v3 07/13] t4014: drop redirections to /dev/null Denton Liu
2019-08-20  7:19   ` [PATCH v3 08/13] t4014: use test_line_count() where possible Denton Liu
2019-08-20  7:19   ` [PATCH v3 09/13] t4014: remove confusing pipe in check_threading() Denton Liu
2019-08-20  7:19   ` [PATCH v3 10/13] t4014: stop losing return codes of git commands Denton Liu
2019-08-20  7:31     ` Denton Liu
2019-08-20 19:04       ` Johannes Sixt
2019-08-20  7:19   ` [PATCH v3 11/13] Doc: add more detail for git-format-patch Denton Liu
2019-08-21 18:26     ` Junio C Hamano
2019-08-20  7:19   ` [PATCH v3 12/13] config/format.txt: specify default value of format.coverLetter Denton Liu
2019-08-20  7:19   ` [PATCH v3 13/13] format-patch: learn --infer-cover-subject option Denton Liu
2019-08-21 19:32     ` Junio C Hamano
2019-08-23 18:15       ` Denton Liu
2019-08-23 18:46         ` Philip Oakley
2019-08-23 20:18           ` Junio C Hamano
2019-08-24  8:03             ` Denton Liu
2019-08-24 13:59               ` Philip Oakley
2019-08-26 14:30                 ` Junio C Hamano
2019-08-26 14:26               ` Junio C Hamano
2019-08-26 16:05                 ` Junio C Hamano
2019-08-22 20:18   ` [PATCH v3 00/13] format-patch: learn --infer-cover-subject option (also t4014 cleanup) Junio C Hamano
2019-08-23 18:19     ` Denton Liu
2019-08-23 20:25       ` Junio C Hamano
2019-08-24  8:25   ` [PATCH 00/13] format-patch: clean up tests and documentation Denton Liu
2019-08-24  8:26     ` [PATCH 01/13] t4014: drop unnecessary blank lines from test cases Denton Liu
2019-08-24  8:26     ` [PATCH 02/13] t4014: s/expected/expect/ Denton Liu
2019-08-24  8:26     ` [PATCH 03/13] t4014: move closing sq onto its own line Denton Liu
2019-08-24  8:26     ` [PATCH 04/13] t4014: use sq for test case names Denton Liu
2019-08-24  8:26     ` [PATCH 05/13] t4014: remove spaces after redirect operators Denton Liu
2019-08-24  8:27     ` [PATCH 06/13] t4014: use indentable here-docs Denton Liu
2019-08-24  8:27     ` [PATCH 07/13] t4014: drop redirections to /dev/null Denton Liu
2019-08-24  8:27     ` [PATCH 08/13] t4014: let sed open its own files Denton Liu
2019-08-26  0:42       ` Eric Sunshine
2019-08-24  8:27     ` [PATCH 09/13] t4014: use test_line_count() where possible Denton Liu
2019-08-24  8:27     ` [PATCH 10/13] t4014: remove confusing pipe in check_threading() Denton Liu
2019-08-24  8:27     ` [PATCH 11/13] t4014: stop losing return codes of git commands Denton Liu
2019-08-24  8:27     ` [PATCH 12/13] Doc: add more detail for git-format-patch Denton Liu
2019-08-26 15:20       ` Junio C Hamano
2019-08-26 16:07         ` Junio C Hamano
2019-08-24  8:27     ` [PATCH 13/13] config/format.txt: specify default value of format.coverLetter Denton Liu
2019-08-24  8:28     ` [PATCH 00/13] format-patch: clean up tests and documentation Denton Liu
2019-08-26 15:21       ` Junio C Hamano
2019-08-27  4:04     ` [PATCH v2 " Denton Liu
2019-08-27  4:04       ` [PATCH v2 01/13] t4014: drop unnecessary blank lines from test cases Denton Liu
2019-08-27  4:04       ` [PATCH v2 02/13] t4014: s/expected/expect/ Denton Liu
2019-08-27  4:04       ` [PATCH v2 03/13] t4014: move closing sq onto its own line Denton Liu
2019-08-27  4:04       ` [PATCH v2 04/13] t4014: use sq for test case names Denton Liu
2019-08-27  4:05       ` [PATCH v2 05/13] t4014: remove spaces after redirect operators Denton Liu
2019-08-27  4:05       ` [PATCH v2 06/13] t4014: use indentable here-docs Denton Liu
2019-08-27  4:05       ` [PATCH v2 07/13] t4014: drop redirections to /dev/null Denton Liu
2019-08-27  4:05       ` [PATCH v2 08/13] t4014: let sed open its own files Denton Liu
2019-08-27  4:05       ` [PATCH v2 09/13] t4014: use test_line_count() where possible Denton Liu
2019-08-27  4:05       ` [PATCH v2 10/13] t4014: remove confusing pipe in check_threading() Denton Liu
2019-08-27  4:05       ` [PATCH v2 11/13] t4014: stop losing return codes of git commands Denton Liu
2019-08-27  4:05       ` [PATCH v2 12/13] Doc: add more detail for git-format-patch Denton Liu
2019-08-27  4:05       ` [PATCH v2 13/13] config/format.txt: specify default value of format.coverLetter Denton Liu
2019-09-04 11:21       ` [PATCH v2 00/13] format-patch: clean up tests and documentation Denton Liu
2019-09-05 19:56         ` Junio C Hamano
2019-09-05 21:40           ` Denton Liu
2019-10-11 19:12   ` [PATCH v4 0/3] format-patch: learn --cover-from-description option Denton Liu
2019-10-11 19:12     ` [PATCH v4 1/3] format-patch: remove erroneous and condition Denton Liu
2019-10-11 19:12     ` [PATCH v4 2/3] format-patch: use enum variables Denton Liu
2019-10-12  2:16       ` Junio C Hamano
2019-10-11 19:12     ` [PATCH v4 3/3] format-patch: teach --cover-from-description option Denton Liu
2019-10-12  2:36       ` Junio C Hamano
2019-10-11 19:23     ` [PATCH v4 4/3] fixup! " Denton Liu
2019-10-12  4:18     ` [PATCH v4 0/3] format-patch: learn " Junio C Hamano
2019-10-14 20:46     ` Denton Liu [this message]
2019-10-14 20:46       ` [PATCH v5 1/3] format-patch: change erroneous and condition Denton Liu
2019-10-15  2:16         ` Junio C Hamano
2019-10-15  3:45           ` Denton Liu
2019-10-14 20:47       ` [PATCH v5 2/3] format-patch: use enum variables Denton Liu
2019-10-14 20:47       ` [PATCH v5 3/3] format-patch: teach --cover-from-description option Denton Liu
2019-10-15  2:25         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1571085952.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=philipoakley@iee.email \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).