Re: [PATCH] git-log --format: Add %B tag with %B(x) option
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:25
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] git-log --format: Add %B tag with %B(x) option · Johannes Gilger <hidden>
Johannes Gilger [off-list ref] writes:
quoted hunk
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index 2a845b1..c04f118 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt@@ -123,6 +123,8 @@ The placeholders are: - '%s': subject - '%f': sanitized subject line, suitable for a filename - '%b': body +- '%B': body without trailing newline +- '%B(x)': body indented with x spaces
First the design issues. Because this will set a precedent for possible future formatting features that take optional parameters, we need to pick the syntax carefully not only for this feature but for the later ones that we haven't invented yet. Let's say that pair of parentheses is a good choice and if later features want to take more than one, it would be reasonable for them to use a comma-separated list, e.g. %Q(1,2,3). I wonder if it is reasonable to invoke print_wrapped_text(), not just limit this feature to indenting. Now, let's look at the implementation.
quoted hunk
diff --git a/pretty.c b/pretty.c index f5983f8..6d530e1 100644 --- a/pretty.c +++ b/pretty.c@@ -735,6 +735,19 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, case 'b': /* body */ strbuf_addstr(sb, msg + c->body_off); return 1; + case 'B': + if (placeholder[1] == '(') { + const char *body = msg + c->body_off; + const char *end = strchr(placeholder + 2, ')'); + if(!end) + return 0;
Style: "if (!end)" I'd sleep better if the syntax checking is done as a separate phase way before this in the codepath.
+ pp_remainder(CMIT_FMT_MEDIUM, &body, sb, atoi(placeholder + 2));
What happens when atoi() fails, or %B(12Q) was given? We tend to use strto[u]l when parsing integers and check for errors.