[v6,0/3] format-patch: learn --infer-cover-subject option (also t4014 cleanup)
mbox series

Message ID cover.1571130298.git.liu.denton@gmail.com
Headers show
Series
  • format-patch: learn --infer-cover-subject option (also t4014 cleanup)
Related show

Message

Denton Liu Oct. 15, 2019, 9:06 a.m. UTC
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 `--infer-cover-subject` option and corresponding
`format.inferCoverSubject` configuration option which will read the
subject from the branch description using the same rules as for a commit
message (that is, it will expect a subject line followed by a blank
line).

This was based on patches 1-3 of an earlier patchset I sent[1].

Changes since v5:

* Remove speculation in log message of 1/3

* Rename pp_from_desc() to prepare_cover_text()

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"

[1]: https://public-inbox.org/git/cover.1558492582.git.liu.denton@gmail.com/

Denton Liu (3):
  format-patch: replace 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 v5:
1:  f89e56545b ! 1:  9d41068e73 format-patch: change erroneous and condition
    @@ Metadata
     Author: Denton Liu <liu.denton@gmail.com>
     
      ## Commit message ##
    -    format-patch: change erroneous and condition
    +    format-patch: replace 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.
    -
    -    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>
    +    is a no-op. Replace this errorneous and condition with a ternary
    +    statement so that it is clear what the configured value is when a
    +    boolean is given.
     
      ## builtin/log.c ##
     @@ builtin/log.c: static int git_format_config(const char *var, const char *value, void *cb)
2:  1fe780b5a4 = 2:  821e706bae format-patch: use enum variables
3:  d5cd34a44b ! 3:  42b4a60fd2 format-patch: teach --cover-from-description option
    @@ 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)
    ++static void prepare_cover_text(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 ***";
    @@ 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_from_desc(&pp, branch_name, &sb, encoding, need_8bit_cte);
    ++	prepare_cover_text(&pp, branch_name, &sb, encoding, need_8bit_cte);
      	fprintf(rev->diffopt.file, "%s\n", sb.buf);
      
      	strbuf_release(&sb);

Comments

Junio C Hamano Oct. 16, 2019, 1:28 a.m. UTC | #1
Thanks.  I think we are ready for 'next'.