diff mbox series

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

Message ID 20181104152232.20671-5-anders@0x63.nu (mailing list archive)
State New, archived
Headers show
Series %(trailers) improvements in pretty format | expand

Commit Message

Anders Waldenborg Nov. 4, 2018, 3:22 p.m. UTC
No functional change intended

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

Comments

Junio C Hamano Nov. 5, 2018, 2:06 a.m. UTC | #1
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;
Anders Waldenborg Nov. 5, 2018, 8:32 a.m. UTC | #2
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?
Junio C Hamano Nov. 6, 2018, 1:46 a.m. UTC | #3
Anders Waldenborg <anders@0x63.nu> 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".
diff mbox series

Patch

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;