All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Kristoffer Haugsbakk <code@khaugsbakk.name>
Cc: git@vger.kernel.org
Subject: [PATCH 2/6] pretty: split oneline and email subject printing
Date: Tue, 19 Mar 2024 20:28:51 -0400	[thread overview]
Message-ID: <20240320002851.GB904136@coredump.intra.peff.net> (raw)
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


  parent reply	other threads:[~2024-03-20  0:28 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-07 19:59 [PATCH 0/3] format-patch: teach `--header-cmd` Kristoffer Haugsbakk
2024-03-07 19:59 ` [PATCH 1/3] log-tree: take ownership of pointer Kristoffer Haugsbakk
2024-03-12  9:29   ` Jeff King
2024-03-12 17:43     ` Kristoffer Haugsbakk
2024-03-13  6:54       ` Jeff King
2024-03-13 17:49         ` Kristoffer Haugsbakk
2024-03-07 19:59 ` [PATCH 2/3] format-patch: teach `--header-cmd` Kristoffer Haugsbakk
2024-03-08 18:30   ` Kristoffer Haugsbakk
2024-03-11 21:29   ` Jean-Noël Avila
2024-03-12  8:13     ` Kristoffer Haugsbakk
2024-03-07 19:59 ` [PATCH 3/3] format-patch: check if header output looks valid Kristoffer Haugsbakk
2024-03-19 18:35 ` [PATCH v2 0/3] format-patch: teach `--header-cmd` Kristoffer Haugsbakk
2024-03-19 18:35   ` [PATCH v2 1/3] revision: add a per-email field to rev-info Kristoffer Haugsbakk
2024-03-19 21:29     ` Jeff King
2024-03-19 21:41       ` Kristoffer Haugsbakk
2024-03-20  0:25       ` Jeff King
2024-03-20  0:27         ` [PATCH 1/6] shortlog: stop setting pp.print_email_subject Jeff King
2024-03-20  0:28         ` Jeff King [this message]
2024-03-22 22:00           ` [PATCH 2/6] pretty: split oneline and email subject printing Kristoffer Haugsbakk
2024-03-20  0:30         ` [PATCH 3/6] pretty: drop print_email_subject flag Jeff King
2024-03-20  0:31         ` [PATCH 4/6] log: do not set up extra_headers for non-email formats Jeff King
2024-03-22 22:04           ` Kristoffer Haugsbakk
2024-03-20  0:35         ` [PATCH 5/6] format-patch: return an allocated string from log_write_email_headers() Jeff King
2024-03-22 22:06           ` Kristoffer Haugsbakk
2024-03-20  0:35         ` [PATCH 6/6] format-patch: simplify after-subject MIME header handling Jeff King
2024-03-22 22:08           ` Kristoffer Haugsbakk
2024-03-20  0:43         ` [PATCH v2 1/3] revision: add a per-email field to rev-info Jeff King
2024-03-22 22:31           ` Kristoffer Haugsbakk
2024-03-22  9:59         ` [PATCH 7/6] format-patch: fix leak of empty header string Jeff King
2024-03-22 10:03           ` Kristoffer Haugsbakk
2024-03-22 16:50           ` Junio C Hamano
2024-03-22 22:16           ` Kristoffer Haugsbakk
2024-03-19 18:35   ` [PATCH v2 2/3] format-patch: teach `--header-cmd` Kristoffer Haugsbakk
2024-03-19 18:35   ` [PATCH v2 3/3] format-patch: check if header output looks valid Kristoffer Haugsbakk

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

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

  git send-email \
    --in-reply-to=20240320002851.GB904136@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=code@khaugsbakk.name \
    --cc=git@vger.kernel.org \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.