Johannes Gilger <heipei@xxxxxxxxxxxx> writes:
> 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.
> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html