git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 0/2] format-patch --rfc=WIP

From
Junio C Hamano <gitster@pobox.com>
Date
Apr 23, 2024, 17:52 UTC
Message-ID
<20240423175234.170434-1-gitster@pobox.com>
In-Reply-To
<20240421185915.1031590-1-gitster@pobox.com>

In the previous iteration, I botched the implementation of the "--no-rfc" case and somehow the test was "lucky" enough to leave uninitialized pieces of memory filled with '\0's and did not catch it.

The proposed log message for the second one has been reworded, and the documentation (obliquely) cautions against abusing the suffix form just to be different.

Junio C Hamano (2):
  format-patch: allow --rfc to optionally take a value, like --rfc=WIP
  format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)]
 Documentation/git-format-patch.txt | 21 ++++++++++++++++-----
 builtin/log.c                      | 27 +++++++++++++++++++++++----
 t/t4014-format-patch.sh            | 28 +++++++++++++++++++++++++++-
 3 files changed, 66 insertions(+), 10 deletions(-)
Range-diff against v3:
1:  7e022c85f7 ! 1:  30798e1211 format-patch: allow --rfc to optionally take a value, like --rfc=WIP
    @@ builtin/log.c: static int subject_prefix_callback(const struct option *opt, cons
     +	const char **rfc = opt->value;
     +
     +	*rfc = opt->value;
    -+	if (!unset)
    ++	if (unset)
    ++		*rfc = NULL;
    ++	else
     +		*rfc = arg ? arg : "RFC";
     +	return 0;
     +}
    @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
      			    N_("cover-from-description-mode"),
      			    N_("generate parts of a cover letter based on a branch's description")),
     @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)
    + 	if (cover_from_description_arg)
      		cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
      
    - 	if (rfc)
    +-	if (rfc)
     -		strbuf_insertstr(&sprefix, 0, "RFC ");
    ++	if (rfc && rfc[0])
     +		strbuf_insertf(&sprefix, 0, "%s ", rfc);
      
      	if (reroll_count) {
2:  474041bdf8 ! 2:  fdbe198cd7 format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)]
    @@ Commit message
     
         In the previous step, the "--rfc" option of "format-patch" learned
         to take an optional string value to prepend to the subject prefix,
    -    so that --rfc=WIP can give "[WIP PATCH]".  This commit shows that
    -    the mechanism can be extended easily to allow "--rfc=-(WIP)" [*1*]
    -    to signal that the extra string is to be appended instead of getting
    -    prepended, resulting in "[PATCH (WIP)]".
    -
    -    Having worked on the patch, I am personally not 100% on board with
    -    this part of the feature myself, and that is why this update is a
    -    separate step from the main "--rfc takes an optional string value"
    -    step.
    -
    -    If a way to prepend an arbitrary adornment is added to the system,
    -    and people can now say "--rfc=RESEND" to produce "[RESEND PATCH]",
    -    those who used to use "[PATCH RESEND]" by tweaking the subject
    -    prefix in other ways [*2*] would do one of three things:
    -
    -     (1) keep using their existing ways and keep sending their message
    -         with the "[RESEND PATCH]" prefix.
    -
    -     (2) rejoice in the new automation, switch to use "--rfc=RESEND",
    -         and start sending their messages with "[RESEND PATCH]" prefix
    -         instead of "[PATCH RESEND]" prefix.
    +    so that --rfc=WIP can give "[WIP PATCH]".
     
    -     (3) complain and demand a way to append instead of prepend so that
    -         they can use an automation to produce "[PATCH RESEND]" prefix.
    +    There may be cases in which the extra string wants to come after the
    +    subject prefix.  Extend the mechanism to allow "--rfc=-(WIP)" [*] to
    +    signal that the extra string is to be appended instead of getting
    +    prepended, resulting in "[PATCH (WIP)]".
     
    -    I do not believe in adding slightly different ways that allow users
    -    to be "original" when such differences do not make the world better
    -    in meaningful ways [*3*], and I expect there are many more folks who
    -    share that sentiment and go to route (2) than those who go to route
    -    (3).  If my expectation is true, it means that this patch goes in a
    -    wrong direction, and I would be happy to drop it.
    +    In the documentation, discourage (ab)using "--rfc=-RFC" to say
    +    "[PATCH RFC]" just to be different, when "[RFC PATCH]" is the norm.
     
         [Footnote]
     
    -     *1* The syntax takes inspiration from Perl's three-arg open syntax
    -         that uses pipes "open fh, '|-', 'cmd'", where the dash signals
    -         "the other stuff comes here".
    -
    -     *2* ... because "--rfc" in released versions does not even take any
    -         string value to prepend, let alone append, to begin with.
    -
    -     *3* https://lore.kernel.org/git/b4d2b3faaf2914b7083327d5a4be3905@manjaro.org/
    -         gathered some stats to observe that "[RFC PATCH]" is more
    -         common than "[PATCH RFC]" by a wide margin, while trying to see
    -         how common "[RESEND PATCH]" (or "[PATCH RESED]") were used (the
    -         answer: much less common).  But it wouldn't have needed to
    -         count "[PATCH RFC]" and "[RFC PATCH]" separately if using a
    -         prefix and not a suffix (or vice versa) were established more
    -         firmly as the standard practice.
    -
    -         It is a fine example that useless diversity making needless
    -         work.
    +     * The syntax takes inspiration from Perl's open syntax that opens
    +       pipes "open fh, '|-', 'cmd'", where the dash signals "the other
    +       stuff comes here".
     
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
    @@ Documentation/git-format-patch.txt: RFC means "Request For Comments"; use this w
      "--rfc=WIP" may also be a useful way to indicate that a patch
      is not complete yet ("WIP" stands for "Work In Progress").
     ++
    -+If the string _<rfc>_ begins with a dash ("`-`"), the rest of the
    -+_<rfc>_ string is appended to the subject prefix instead, e.g.,
    -+`--rfc='-(WIP)'` results in "PATCH (WIP)".
    ++If the convention of the receiving community for a particular extra
    ++string is to have it _after_ the subject prefix, the string _<rfc>_
    ++can be prefixed with a dash ("`-`") to signal that the the rest of
    ++the _<rfc>_ string should be appended to the subject prefix instead,
    ++e.g., `--rfc='-(WIP)'` results in "PATCH (WIP)".
      
      -v <n>::
      --reroll-count=<n>::
    @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
      	if (cover_from_description_arg)
      		cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
      
    --	if (rfc)
    +-	if (rfc && rfc[0])
     -		strbuf_insertf(&sprefix, 0, "%s ", rfc);
    -+	if (rfc) {
    ++	if (rfc && rfc[0]) {
     +		if (rfc[0] == '-')
     +			strbuf_addf(&sprefix, " %s", rfc + 1);
     +		else
-- 
2.45.0-rc0-3-g00e10ef10e
Previous: Junio C HamanoNext: Junio C Hamano
Message 17 of 24 in “format-patch: allow --rfc to optionally take a value, like --rfc=WIP”
  1. format-patch: allow --rfc to optionally take a value, like --rfc=WIPJunio C Hamano, Apr 18, 2024
  2. Dragan SimicApr 19, 2024
  3. Phillip WoodApr 19, 2024
  4. Junio C HamanoApr 19, 2024
  5. Dragan SimicApr 21, 2024
  6. Jeff KingApr 19, 2024
  7. Junio C HamanoApr 19, 2024
  8. format-patch: allow --rfc to optionally take a value, like --rfc=WIPJunio C Hamano, Apr 19, 2024
  9. Phillip WoodApr 21, 2024
  10. Junio C HamanoApr 21, 2024
  11. 0/2 format-patch --rfc=WIPJunio C Hamano, Apr 21, 2024
  12. 2/2 format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)]Junio C Hamano, Apr 21, 2024
  13. Dragan SimicApr 21, 2024
  14. Phillip WoodApr 24, 2024
  15. Dragan SimicApr 24, 2024
  16. 1/2 format-patch: allow --rfc to optionally take a value, like --rfc=WIPJunio C Hamano, Apr 21, 2024
  17. 0/2 format-patch --rfc=WIPJunio C Hamano, Apr 23, 2024
  18. 1/2 format-patch: allow --rfc to optionally take a value, like --rfc=WIPJunio C Hamano, Apr 23, 2024
  19. Phillip WoodApr 24, 2024
  20. 2/2 format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)]Junio C Hamano, Apr 23, 2024
  21. Phillip WoodApr 24, 2024
  22. Junio C HamanoApr 24, 2024
  23. Dragan SimicApr 24, 2024
  24. Dragan SimicApr 24, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.