git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Rast <trast@student.ethz.ch>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Jef Driesen <jefdriesen@hotmail.com>,
	Nanako Shiraishi <nanako3@lavabit.com>, <git@vger.kernel.org>
Subject: [PATCH v3 1/5] Refactor pretty_print_commit arguments into a struct
Date: Fri, 16 Oct 2009 16:20:33 +0200	[thread overview]
Message-ID: <9d3d0f0a6126afc86689138adf58ac7a12c43858.1255701207.git.trast@student.ethz.ch> (raw)
In-Reply-To: <cover.1255701207.git.trast@student.ethz.ch>

pretty_print_commit() has a bunch of rarely-used arguments, and
introducing more of them requires yet another update of all the call
sites.  Refactor most of them into a struct to make future extensions
easier.

The ones that stay "plain" arguments were chosen on the grounds that
all callers put real arguments there, whereas some callers have 0/NULL
for all arguments that were factored into the struct.

We declare the struct 'const' to ensure none of the callers are bitten
by the changed (no longer call-by-value) semantics.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 builtin-branch.c      |    3 ++-
 builtin-checkout.c    |    3 ++-
 builtin-log.c         |    3 ++-
 builtin-merge.c       |    7 +++++--
 builtin-rev-list.c    |    7 ++++---
 builtin-shortlog.c    |    9 ++++++---
 builtin-show-branch.c |    4 ++--
 commit.h              |   19 +++++++++++++------
 log-tree.c            |   20 ++++++++++----------
 pretty.c              |   27 ++++++++++++++-------------
 10 files changed, 60 insertions(+), 42 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 9f57992..05e876e 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -387,8 +387,9 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
 
 		commit = item->commit;
 		if (commit && !parse_commit(commit)) {
+			struct pretty_print_context ctx = {0};
 			pretty_print_commit(CMIT_FMT_ONELINE, commit,
-					    &subject, 0, NULL, NULL, 0, 0);
+					    &subject, &ctx);
 			sub = subject.buf;
 		}
 
diff --git a/builtin-checkout.c b/builtin-checkout.c
index d050c37..075a49f 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -302,8 +302,9 @@ static void show_local_changes(struct object *head)
 static void describe_detached_head(char *msg, struct commit *commit)
 {
 	struct strbuf sb = STRBUF_INIT;
+	struct pretty_print_context ctx = {0};
 	parse_commit(commit);
-	pretty_print_commit(CMIT_FMT_ONELINE, commit, &sb, 0, NULL, NULL, 0, 0);
+	pretty_print_commit(CMIT_FMT_ONELINE, commit, &sb, &ctx);
 	fprintf(stderr, "%s %s... %s\n", msg,
 		find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV), sb.buf);
 	strbuf_release(&sb);
diff --git a/builtin-log.c b/builtin-log.c
index 25e21ed..207a361 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1304,8 +1304,9 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 
 		if (verbose) {
 			struct strbuf buf = STRBUF_INIT;
+			struct pretty_print_context ctx = {0};
 			pretty_print_commit(CMIT_FMT_ONELINE, commit,
-			                    &buf, 0, NULL, NULL, 0, 0);
+					    &buf, &ctx);
 			printf("%c %s %s\n", sign,
 			       sha1_to_hex(commit->object.sha1), buf.buf);
 			strbuf_release(&buf);
diff --git a/builtin-merge.c b/builtin-merge.c
index b6b8428..c69a305 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -264,6 +264,7 @@ static void squash_message(void)
 	struct strbuf out = STRBUF_INIT;
 	struct commit_list *j;
 	int fd;
+	struct pretty_print_context ctx = {0};
 
 	printf("Squash commit -- not updating HEAD\n");
 	fd = open(git_path("SQUASH_MSG"), O_WRONLY | O_CREAT, 0666);
@@ -285,13 +286,15 @@ static void squash_message(void)
 	if (prepare_revision_walk(&rev))
 		die("revision walk setup failed");
 
+	ctx.abbrev = rev.abbrev;
+	ctx.date_mode = rev.date_mode;
+
 	strbuf_addstr(&out, "Squashed commit of the following:\n");
 	while ((commit = get_revision(&rev)) != NULL) {
 		strbuf_addch(&out, '\n');
 		strbuf_addf(&out, "commit %s\n",
 			sha1_to_hex(commit->object.sha1));
-		pretty_print_commit(rev.commit_format, commit, &out, rev.abbrev,
-			NULL, NULL, rev.date_mode, 0);
+		pretty_print_commit(rev.commit_format, commit, &out, &ctx);
 	}
 	if (write(fd, out.buf, out.len) < 0)
 		die_errno("Writing SQUASH_MSG");
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 4ba1c12..42cc8d8 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -96,9 +96,10 @@ static void show_commit(struct commit *commit, void *data)
 
 	if (revs->verbose_header && commit->buffer) {
 		struct strbuf buf = STRBUF_INIT;
-		pretty_print_commit(revs->commit_format, commit,
-				    &buf, revs->abbrev, NULL, NULL,
-				    revs->date_mode, 0);
+		struct pretty_print_context ctx = {0};
+		ctx.abbrev = revs->abbrev;
+		ctx.date_mode = revs->date_mode;
+		pretty_print_commit(revs->commit_format, commit, &buf, &ctx);
 		if (revs->graph) {
 			if (buf.len) {
 				if (revs->commit_format != CMIT_FMT_ONELINE)
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 4d4a3c8..8aa63c7 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -158,9 +158,12 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
 		    sha1_to_hex(commit->object.sha1));
 	if (log->user_format) {
 		struct strbuf buf = STRBUF_INIT;
-
-		pretty_print_commit(CMIT_FMT_USERFORMAT, commit, &buf,
-			DEFAULT_ABBREV, "", "", DATE_NORMAL, 0);
+		struct pretty_print_context ctx = {0};
+		ctx.abbrev = DEFAULT_ABBREV;
+		ctx.subject = "";
+		ctx.after_subject = "";
+		ctx.date_mode = DATE_NORMAL;
+		pretty_print_commit(CMIT_FMT_USERFORMAT, commit, &buf, &ctx);
 		insert_one_record(log, author, buf.buf);
 		strbuf_release(&buf);
 		return;
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index be95930..9f13caa 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -293,8 +293,8 @@ static void show_one_commit(struct commit *commit, int no_name)
 	struct commit_name *name = commit->util;
 
 	if (commit->object.parsed) {
-		pretty_print_commit(CMIT_FMT_ONELINE, commit,
-				    &pretty, 0, NULL, NULL, 0, 0);
+		struct pretty_print_context ctx = {0};
+		pretty_print_commit(CMIT_FMT_ONELINE, commit, &pretty, &ctx);
 		pretty_str = pretty.buf;
 	}
 	if (!prefixcmp(pretty_str, "[PATCH] "))
diff --git a/commit.h b/commit.h
index f4fc5c5..011766d 100644
--- a/commit.h
+++ b/commit.h
@@ -63,6 +63,15 @@ enum cmit_fmt {
 	CMIT_FMT_UNSPECIFIED,
 };
 
+struct pretty_print_context
+{
+	int abbrev;
+	const char *subject;
+	const char *after_subject;
+	enum date_mode date_mode;
+	int need_8bit_cte;
+};
+
 extern int non_ascii(int);
 extern int has_non_ascii(const char *text);
 struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
@@ -71,12 +80,10 @@ enum cmit_fmt {
 extern void get_commit_format(const char *arg, struct rev_info *);
 extern void format_commit_message(const struct commit *commit,
 				  const void *format, struct strbuf *sb,
-				  enum date_mode dmode);
-extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit*,
-                                struct strbuf *,
-                                int abbrev, const char *subject,
-                                const char *after_subject, enum date_mode,
-				int need_8bit_cte);
+				  const struct pretty_print_context *context);
+extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
+				struct strbuf *sb,
+				const struct pretty_print_context *context);
 void pp_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 		   const char *line, enum date_mode dmode,
 		   const char *encoding);
diff --git a/log-tree.c b/log-tree.c
index f7d54f2..f57487f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -277,10 +277,9 @@ void show_log(struct rev_info *opt)
 	struct strbuf msgbuf = STRBUF_INIT;
 	struct log_info *log = opt->loginfo;
 	struct commit *commit = log->commit, *parent = log->parent;
-	int abbrev = opt->diffopt.abbrev;
 	int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;
-	const char *subject = NULL, *extra_headers = opt->extra_headers;
-	int need_8bit_cte = 0;
+	const char *extra_headers = opt->extra_headers;
+	struct pretty_print_context ctx = {0};
 
 	opt->loginfo = NULL;
 	if (!opt->verbose_header) {
@@ -347,8 +346,8 @@ void show_log(struct rev_info *opt)
 	 */
 
 	if (opt->commit_format == CMIT_FMT_EMAIL) {
-		log_write_email_headers(opt, commit, &subject, &extra_headers,
-					&need_8bit_cte);
+		log_write_email_headers(opt, commit, &ctx.subject, &extra_headers,
+					&ctx.need_8bit_cte);
 	} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
 		fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
 		if (opt->commit_format != CMIT_FMT_ONELINE)
@@ -405,11 +404,12 @@ void show_log(struct rev_info *opt)
 	/*
 	 * And then the pretty-printed message itself
 	 */
-	if (need_8bit_cte >= 0)
-		need_8bit_cte = has_non_ascii(opt->add_signoff);
-	pretty_print_commit(opt->commit_format, commit, &msgbuf,
-			    abbrev, subject, extra_headers, opt->date_mode,
-			    need_8bit_cte);
+	if (ctx.need_8bit_cte >= 0)
+		ctx.need_8bit_cte = has_non_ascii(opt->add_signoff);
+	ctx.date_mode = opt->date_mode;
+	ctx.abbrev = opt->diffopt.abbrev;
+	ctx.after_subject = extra_headers;
+	pretty_print_commit(opt->commit_format, commit, &msgbuf, &ctx);
 
 	if (opt->add_signoff)
 		append_signoff(&msgbuf, opt->add_signoff);
diff --git a/pretty.c b/pretty.c
index f5983f8..d6d57eb 100644
--- a/pretty.c
+++ b/pretty.c
@@ -442,7 +442,7 @@ struct chunk {
 
 struct format_commit_context {
 	const struct commit *commit;
-	enum date_mode dmode;
+	const struct pretty_print_context *pretty_ctx;
 	unsigned commit_header_parsed:1;
 	unsigned commit_message_parsed:1;
 
@@ -711,11 +711,11 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	case 'a':	/* author ... */
 		return format_person_part(sb, placeholder[1],
 				   msg + c->author.off, c->author.len,
-				   c->dmode);
+				   c->pretty_ctx->date_mode);
 	case 'c':	/* committer ... */
 		return format_person_part(sb, placeholder[1],
 				   msg + c->committer.off, c->committer.len,
-				   c->dmode);
+				   c->pretty_ctx->date_mode);
 	case 'e':	/* encoding */
 		strbuf_add(sb, msg + c->encoding.off, c->encoding.len);
 		return 1;
@@ -741,13 +741,13 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 
 void format_commit_message(const struct commit *commit,
 			   const void *format, struct strbuf *sb,
-			   enum date_mode dmode)
+			   const struct pretty_print_context *pretty_ctx)
 {
 	struct format_commit_context context;
 
 	memset(&context, 0, sizeof(context));
 	context.commit = commit;
-	context.dmode = dmode;
+	context.pretty_ctx = pretty_ctx;
 	strbuf_expand(sb, format, format_commit_item, &context);
 }
 
@@ -900,18 +900,18 @@ void pp_remainder(enum cmit_fmt fmt,
 }
 
 void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
-			 struct strbuf *sb, int abbrev,
-			 const char *subject, const char *after_subject,
-			 enum date_mode dmode, int need_8bit_cte)
+			 struct strbuf *sb,
+			 const struct pretty_print_context *context)
 {
 	unsigned long beginning_of_body;
 	int indent = 4;
 	const char *msg = commit->buffer;
 	char *reencoded;
 	const char *encoding;
+	int need_8bit_cte = context->need_8bit_cte;
 
 	if (fmt == CMIT_FMT_USERFORMAT) {
-		format_commit_message(commit, user_format, sb, dmode);
+		format_commit_message(commit, user_format, sb, context);
 		return;
 	}
 
@@ -946,8 +946,9 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		}
 	}
 
-	pp_header(fmt, abbrev, dmode, encoding, commit, &msg, sb);
-	if (fmt != CMIT_FMT_ONELINE && !subject) {
+	pp_header(fmt, context->abbrev, context->date_mode, encoding,
+		  commit, &msg, sb);
+	if (fmt != CMIT_FMT_ONELINE && !context->subject) {
 		strbuf_addch(sb, '\n');
 	}
 
@@ -956,8 +957,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 
 	/* These formats treat the title line specially. */
 	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
-		pp_title_line(fmt, &msg, sb, subject,
-			      after_subject, encoding, need_8bit_cte);
+		pp_title_line(fmt, &msg, sb, context->subject,
+			      context->after_subject, encoding, need_8bit_cte);
 
 	beginning_of_body = sb->len;
 	if (fmt != CMIT_FMT_ONELINE)
-- 
1.6.5.116.gfe4b9

  reply	other threads:[~2009-10-16 14:25 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-12 15:47 git stash list with more than 10 items? Jef Driesen
2009-10-12 17:52 ` Jeff King
2009-10-12 19:37   ` [PATCH] git-stash documentation: mention default options for 'list' Miklos Vajna
2009-10-12 19:39     ` Jeff King
2009-10-12 21:06   ` [RFC PATCH 0/5] Pretty formats for reflog data Thomas Rast
2009-10-12 21:06     ` [RFC PATCH 1/5] reflog-walk: refactor the branch@{num} formatting Thomas Rast
2009-10-12 21:06     ` [RFC PATCH 2/5] Introduce new pretty formats %g and %G for reflog information Thomas Rast
2009-10-14  4:59       ` Jeff King
2009-10-14  9:58         ` Thomas Rast
2009-10-14  9:13       ` Junio C Hamano
2009-10-12 21:06     ` [RFC PATCH 3/5] stash: Use new %g/%G formats instead of sed Thomas Rast
2009-10-14  5:00       ` Jeff King
2009-10-12 21:06     ` [RFC PATCH 4/5] stash list: drop the default limit of 10 stashes Thomas Rast
2009-10-14  5:02       ` Jeff King
2009-10-12 21:06     ` [RFC PATCH 5/5] stash: change built-in ref to 'stash' instead of 'refs/stash' Thomas Rast
2009-10-14  5:06       ` Jeff King
2009-10-15 22:41         ` [PATCH v2 0/5] Pretty formats for reflog data Thomas Rast
2009-10-15 22:41           ` [PATCH v2 1/5] Refactor pretty_print_commit arguments into a struct Thomas Rast
2009-10-15 22:41           ` [PATCH v2 2/5] reflog-walk: refactor the branch@{num} formatting Thomas Rast
2009-10-15 22:41           ` [PATCH v2 3/5] Introduce new pretty formats %g[sdD] for reflog information Thomas Rast
2009-10-16  5:32             ` Jeff King
2009-10-16  8:50               ` Thomas Rast
2009-10-16 14:20               ` [PATCH v3 0/5] Pretty formats for reflog data Thomas Rast
2009-10-16 14:20                 ` Thomas Rast [this message]
2009-10-17 17:05                   ` [PATCH v3 1/5] Refactor pretty_print_commit arguments into a struct Junio C Hamano
2009-10-18 18:51                     ` Thomas Rast
2009-10-18 22:47                       ` Junio C Hamano
2009-10-19 15:48                         ` [PATCH v4 0/5] Pretty formats for reflog data Thomas Rast
2009-10-19 15:48                           ` [PATCH v4 1/5] Refactor pretty_print_commit arguments into a struct Thomas Rast
2009-10-19 15:48                           ` [PATCH v4 2/5] reflog-walk: refactor the branch@{num} formatting Thomas Rast
2009-10-19 15:48                           ` [PATCH v4 3/5] Introduce new pretty formats %g[sdD] for reflog information Thomas Rast
2009-10-19 15:48                           ` [PATCH v4 4/5] stash list: use new %g formats instead of sed Thomas Rast
2009-10-19 15:48                           ` [PATCH v4 5/5] stash list: drop the default limit of 10 stashes Thomas Rast
2009-10-16 14:20                 ` [PATCH v3 2/5] reflog-walk: refactor the branch@{num} formatting Thomas Rast
2009-10-16 14:20                 ` [PATCH v3 3/5] Introduce new pretty formats %g[sdD] for reflog information Thomas Rast
2009-10-17 14:48                   ` [PATCH v3.1 " Thomas Rast
2009-10-17 15:06                     ` Jakub Narebski
2009-10-18  7:18                     ` Jeff King
2009-10-18 10:34                       ` Nanako Shiraishi
2009-10-16 14:20                 ` [PATCH v3 4/5] stash list: use new %g formats instead of sed Thomas Rast
2009-10-16 14:20                 ` [PATCH v3 5/5] stash list: drop the default limit of 10 stashes Thomas Rast
2009-10-17  0:50                 ` [PATCH v3 0/5] Pretty formats for reflog data Junio C Hamano
2009-10-17  1:18                 ` Jeff King
2009-10-15 22:41           ` [PATCH v2 4/5] stash list: use new %g formats instead of sed Thomas Rast
2009-10-15 22:41           ` [PATCH v2 5/5] stash list: drop the default limit of 10 stashes Thomas Rast
2009-10-16  5:20           ` [PATCH v2 0/5] Pretty formats for reflog data Jeff King
2009-10-16  9:00             ` Jakub Narebski
2009-10-12 21:37     ` [RFC PATCH " Jeff King
2009-10-12 21:52       ` Thomas Rast

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=9d3d0f0a6126afc86689138adf58ac7a12c43858.1255701207.git.trast@student.ethz.ch \
    --to=trast@student.ethz.ch \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jefdriesen@hotmail.com \
    --cc=nanako3@lavabit.com \
    --cc=peff@peff.net \
    /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 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 NNTP newsgroup(s).