Re: [PATCH v2 4/5] pretty: extract fundamental placeholders to separate function

[Junio C Hamano <gitster@pobox.com>]

Anders Waldenborg <anders@0x63.nu> writes:

> No functional change intended
>
> Signed-off-by: Anders Waldenborg <anders@0x63.nu>
> ---
>  pretty.c | 37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)

I do not think "fundamental" is the best name for this, but I agree
that it would be useful to split the helpers into one that is
"constant across commits" and the other one that is "per commit".

> diff --git a/pretty.c b/pretty.c
> index f87ba4f18..9fdddce9d 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1074,6 +1074,27 @@ static int match_placeholder_arg(const char *to_parse, const char *candidate,
>  	return 0;
>  }
>  
> +static size_t format_fundamental(struct strbuf *sb, /* in UTF-8 */
> +				 const char *placeholder,
> +				 void *context)
> +{
> +	int ch;
> +
> +	switch (placeholder[0]) {
> +	case 'n':		/* newline */
> +		strbuf_addch(sb, '\n');
> +		return 1;
> +	case 'x':
> +		/* %x00 == NUL, %x0a == LF, etc. */
> +		ch = hex2chr(placeholder + 1);
> +		if (ch < 0)
> +			return 0;
> +		strbuf_addch(sb, ch);
> +		return 3;
> +	}
> +	return 0;
> +}
> +
>  static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>  				const char *placeholder,
>  				void *context)
> @@ -1083,9 +1104,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>  	const char *msg = c->message;
>  	struct commit_list *p;
>  	const char *arg;
> -	int ch;
> +	size_t res;
>  
>  	/* these are independent of the commit */
> +	res = format_fundamental(sb, placeholder, NULL);
> +	if (res)
> +		return res;
> +
>  	switch (placeholder[0]) {
>  	case 'C':
>  		if (starts_with(placeholder + 1, "(auto)")) {
> @@ -1104,16 +1129,6 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
>  			 */
>  			return ret;
>  		}
> -	case 'n':		/* newline */
> -		strbuf_addch(sb, '\n');
> -		return 1;
> -	case 'x':
> -		/* %x00 == NUL, %x0a == LF, etc. */
> -		ch = hex2chr(placeholder + 1);
> -		if (ch < 0)
> -			return 0;
> -		strbuf_addch(sb, ch);
> -		return 3;
>  	case 'w':
>  		if (placeholder[1] == '(') {
>  			unsigned long width = 0, indent1 = 0, indent2 = 0;