Thread (50 messages) flat view 50 messages, 13 authors, 2016-06-15

Re: [PATCH] Builtin-commit: show on which branch a commit was added

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:19

Possibly related (same subject, not in this thread)

Pieter de Bie [off-list ref] writes:
This outputs the current branch on which a commit was created, just for
reference. For example:

	Created commit 6d42875 on master: Fix submodule invalid command error

This also reminds the committer when he is on a detached HEAD:

	Created commit 02a7172 on detached HEAD: Also do this for 'git commit --amend'
Given the recent "reminder" discussion, I suspect people without $PS1 set
to show the current branch would like this, majority of others would be
neutral, while some may actively hate it for cluttering the output even
more.  But I also suspect the initial annoyance the third camp may feel
will pass rather quickly after they get used to seeing these.
quoted hunk ↗ jump to hunk
diff --git a/builtin-commit.c b/builtin-commit.c
index 8165bb3..a82483d 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -878,10 +878,31 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	return commitable ? 0 : 1;
 }
 
+static char* get_commit_format_string()
Style.

	static char *get_commit_format_string(void)
+{
+	unsigned char sha[20];
+	const char* head = resolve_ref("HEAD", sha, 0, NULL);
Style.

	const char *head = ...
...
+	else if (!prefixcmp(head, "refs/heads/")) {
+		strbuf_addstr(&buf, " on ");
+		strbuf_addstr(&buf, head + 11);
Isn't this function crafting a format string for format_commit_message()?
What happens if your branch name has % in it?
+	}
+	strbuf_addstr(&buf, ": %s");
+
+	return buf.buf;
API violation, I think; see strbuf_detach().
+}
+
 static void print_summary(const char *prefix, const unsigned char *sha1)
 {
 	struct rev_info rev;
 	struct commit *commit;
+	char* format = get_commit_format_string();
Style.

	char *format = ...
quoted hunk ↗ jump to hunk
@@ -910,10 +931,11 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
 
 	if (!log_tree_commit(&rev, commit)) {
 		struct strbuf buf = STRBUF_INIT;
-		format_commit_message(commit, "%h: %s", &buf, DATE_NORMAL);
+		format_commit_message(commit, format + 7, &buf, DATE_NORMAL);
		printf("%s\n", buf.buf);
		strbuf_release(&buf);
I somehow suspect it might be much simpler, more contained and robust if you:

 (1) chuck get_commit_format_string(), and leave all the existing code as-is;

 (2) format "%h: %s" into buf here;

 (3) call resolve_ref(HEAD) here to see if you are on detached HEAD (or
     otherwise what branch you are on) after (2),

 (4) find the first ':' in buf.buf and do your "on HEAD"/"on master"
     magic, using the result from (3).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help