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

2018-11-05 Thread Junio C Hamano
Anders Waldenborg  writes:

> Junio C Hamano writes:
>> 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".
>
> Any suggestions for a better name?
>
> standalone? simple? invariant? free?

If these are like %n for LF or %09 for HT, perhaps they are
constants or "literals".


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

2018-11-05 Thread Anders Waldenborg


Junio C Hamano writes:
> 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".

Any suggestions for a better name?

standalone? simple? invariant? free?



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

2018-11-04 Thread Junio C Hamano
Anders Waldenborg  writes:

> No functional change intended
>
> Signed-off-by: Anders Waldenborg 
> ---
>  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;