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

[PATCH 2/6] pretty: split oneline and email subject printing

From
Jeff King <peff@peff.net>
Date
Mar 20, 2024, 00:28 UTC
Message-ID
<20240320002851.GB904136@coredump.intra.peff.net>
In-Reply-To
<20240320002555.GB903718@coredump.intra.peff.net>

The pp_title_line() function is used for two formats: the oneline format and the subject line of the email format. But most of the logic in the function does not make any sense for oneline; it is about special formatting of email headers.

Lumping the two formats together made sense long ago in 4234a76167 (Extend --pretty=oneline to cover the first paragraph, 2007-06-11), when there was a lot of manual logic to paste lines together. But later, 88c44735ab (pretty: factor out format_subject(), 2008-12-27) pulled that logic into its own function.

We can implement the oneline format by just calling that one function. This makes the intention of the code much more clear, as we know we only need to worry about those extra email options when dealing with actual email.

While the intent here is cleanup, it is possible to trigger these cases in practice by running format-patch with an explicit --oneline option. But if you did, the results are basically nonsense. For example, with the preserve_subject flag:

  $ printf "%s\n" one two three | git commit --allow-empty -F -
  $ git format-patch -1 --stdout -k | grep ^Subject
  Subject: =?UTF-8?q?one=0Atwo=0Athree?=
  $ git format-patch -1 --stdout -k --oneline --no-signature
  2af7fbe one
  two
  three
Or with extra headers:
  $ git format-patch -1 --stdout --cc=me --oneline --no-signature
  2af7fbe one two three
  Cc: me

So I'd actually consider this to be an improvement, though you are probably crazy to use other formats with format-patch in the first place (arguably it should forbid non-email formats entirely, but that's a bigger change).

As a bonus, it eliminates some pointless extra allocations for the oneline output. The email code, since it has to deal with wrapping, formats into an extra auxiliary buffer. The speedup is tiny, though like "rev-list --no-abbrev --format=oneline" seems to improve by a consistent 1-2% for me.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/log.c |  2 +-
 pretty.c      | 22 ++++++++++++----------
 pretty.h      |  8 ++++----
 3 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index e5da0d1043..89cce9c29d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1297,7 +1297,7 @@ static void prepare_cover_text(struct pretty_print_context *pp,
 		subject = subject_sb.buf;
 
 do_pp:
-	pp_title_line(pp, &subject, sb, encoding, need_8bit_cte);
+	pp_email_subject(pp, &subject, sb, encoding, need_8bit_cte);
 	pp_remainder(pp, &body, sb, 0);
 
 	strbuf_release(&description_sb);
diff --git a/pretty.c b/pretty.c
index bdbed4295a..be0f2f566d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -2077,11 +2077,11 @@ static void pp_header(struct pretty_print_context *pp,
 	}
 }
 
-void pp_title_line(struct pretty_print_context *pp,
-		   const char **msg_p,
-		   struct strbuf *sb,
-		   const char *encoding,
-		   int need_8bit_cte)
+void pp_email_subject(struct pretty_print_context *pp,
+		      const char **msg_p,
+		      struct strbuf *sb,
+		      const char *encoding,
+		      int need_8bit_cte)
 {
 	static const int max_length = 78; /* per rfc2047 */
 	struct strbuf title;
@@ -2126,9 +2126,8 @@ void pp_title_line(struct pretty_print_context *pp,
 	if (pp->after_subject) {
 		strbuf_addstr(sb, pp->after_subject);
 	}
-	if (cmit_fmt_is_mail(pp->fmt)) {
-		strbuf_addch(sb, '\n');
-	}
+
+	strbuf_addch(sb, '\n');
 
 	if (pp->in_body_headers.nr) {
 		int i;
@@ -2328,8 +2327,11 @@ void pretty_print_commit(struct pretty_print_context *pp,
 	msg = skip_blank_lines(msg);
 
 	/* These formats treat the title line specially. */
-	if (pp->fmt == CMIT_FMT_ONELINE || cmit_fmt_is_mail(pp->fmt))
-		pp_title_line(pp, &msg, sb, encoding, need_8bit_cte);
+	if (pp->fmt == CMIT_FMT_ONELINE) {
+		msg = format_subject(sb, msg, " ");
+		strbuf_addch(sb, '\n');
+	} else if (cmit_fmt_is_mail(pp->fmt))
+		pp_email_subject(pp, &msg, sb, encoding, need_8bit_cte);
 
 	beginning_of_body = sb->len;
 	if (pp->fmt != CMIT_FMT_ONELINE)
diff --git a/pretty.h b/pretty.h
index 421209e9ec..d4ff79deb3 100644
--- a/pretty.h
+++ b/pretty.h
@@ -96,13 +96,13 @@ void pp_user_info(struct pretty_print_context *pp, const char *what,
 			const char *encoding);
 
 /*
- * Format title line of commit message taken from "msg_p" and
+ * Format subject line of commit message taken from "msg_p" and
  * put it into "sb".
  * First line of "msg_p" is also affected.
  */
-void pp_title_line(struct pretty_print_context *pp, const char **msg_p,
-			struct strbuf *sb, const char *encoding,
-			int need_8bit_cte);
+void pp_email_subject(struct pretty_print_context *pp, const char **msg_p,
+		      struct strbuf *sb, const char *encoding,
+		      int need_8bit_cte);
 
 /*
  * Get current state of commit message from "msg_p" and continue formatting
-- 
2.44.0.643.g35f318e502
Previous: Jeff KingNext: Kristoffer Haugsbakk
Message 18 of 34 in “format-patch: teach `--header-cmd`”
  1. 0/3 format-patch: teach `--header-cmd`Kristoffer Haugsbakk, Mar 7, 2024
  2. 1/3 log-tree: take ownership of pointerKristoffer Haugsbakk, Mar 7, 2024
  3. Jeff KingMar 12, 2024
  4. Kristoffer HaugsbakkMar 12, 2024
  5. Jeff KingMar 13, 2024
  6. Kristoffer HaugsbakkMar 13, 2024
  7. 2/3 format-patch: teach `--header-cmd`Kristoffer Haugsbakk, Mar 7, 2024
  8. Kristoffer HaugsbakkMar 8, 2024
  9. Jean-Noël AvilaMar 11, 2024
  10. Kristoffer HaugsbakkMar 12, 2024
  11. 3/3 format-patch: check if header output looks validKristoffer Haugsbakk, Mar 7, 2024
  12. 0/3 format-patch: teach `--header-cmd`Kristoffer Haugsbakk, Mar 19, 2024
  13. 1/3 revision: add a per-email field to rev-infoKristoffer Haugsbakk, Mar 19, 2024
  14. Jeff KingMar 19, 2024
  15. Kristoffer HaugsbakkMar 19, 2024
  16. Jeff KingMar 20, 2024
  17. 1/6 shortlog: stop setting pp.print_email_subjectJeff King, Mar 20, 2024
  18. 2/6 pretty: split oneline and email subject printingJeff King, Mar 20, 2024
  19. Kristoffer HaugsbakkMar 22, 2024
  20. 3/6 pretty: drop print_email_subject flagJeff King, Mar 20, 2024
  21. 4/6 log: do not set up extra_headers for non-email formatsJeff King, Mar 20, 2024
  22. Kristoffer HaugsbakkMar 22, 2024
  23. 5/6 format-patch: return an allocated string from log_write_email_headers()Jeff King, Mar 20, 2024
  24. Kristoffer HaugsbakkMar 22, 2024
  25. 6/6 format-patch: simplify after-subject MIME header handlingJeff King, Mar 20, 2024
  26. Kristoffer HaugsbakkMar 22, 2024
  27. Jeff KingMar 20, 2024
  28. Kristoffer HaugsbakkMar 22, 2024
  29. 7/6 format-patch: fix leak of empty header stringJeff King, Mar 22, 2024
  30. Kristoffer HaugsbakkMar 22, 2024
  31. Junio C HamanoMar 22, 2024
  32. Kristoffer HaugsbakkMar 22, 2024
  33. 2/3 format-patch: teach `--header-cmd`Kristoffer Haugsbakk, Mar 19, 2024
  34. 3/3 format-patch: check if header output looks validKristoffer Haugsbakk, Mar 19, 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.