[PATCH 8/8] pretty: refactor parsing of decoration options

["Martin Ågren" <martin.agren@gmail.com>]

After having spotted "%(decorate", we see if there's a ':' and, if so,
reach out to `parse_decoration_options()`. We then verify there's a
closing ')' before actually considering the placeholder valid. Pull the
handling of ':' and ')' into `parse_decoration_options()` so that it's
more of a one-stop shop for handling everything after "%(decorate". Let
this include freeing up resources in the error path to make it really
easy to use this function.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 pretty.c | 52 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/pretty.c b/pretty.c
index ddc7fd6aab..d5a8ceb7ef 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1430,17 +1430,6 @@ static int parse_decoration_option(const char **arg,
 	return 0;
 }
 
-static void parse_decoration_options(const char **arg,
-				     struct decoration_options *opts)
-{
-	while (parse_decoration_option(arg, "prefix", &opts->prefix) ||
-	       parse_decoration_option(arg, "suffix", &opts->suffix) ||
-	       parse_decoration_option(arg, "separator", &opts->separator) ||
-	       parse_decoration_option(arg, "pointer", &opts->pointer) ||
-	       parse_decoration_option(arg, "tag", &opts->tag))
-		;
-}
-
 static void free_decoration_options(const struct decoration_options *opts)
 {
 	free(opts->prefix);
@@ -1450,6 +1439,30 @@ static void free_decoration_options(const struct decoration_options *opts)
 	free(opts->tag);
 }
 
+static int parse_decoration_options(const char **arg,
+				    struct decoration_options *opts)
+{
+	memset(opts, 0, sizeof(*opts));
+
+	if (**arg == ':') {
+		(*arg)++;
+		while (parse_decoration_option(arg, "prefix", &opts->prefix) ||
+		       parse_decoration_option(arg, "suffix", &opts->suffix) ||
+		       parse_decoration_option(arg, "separator", &opts->separator) ||
+		       parse_decoration_option(arg, "pointer", &opts->pointer) ||
+		       parse_decoration_option(arg, "tag", &opts->tag))
+			;
+	}
+
+	if (**arg != ')') {
+		free_decoration_options(opts);
+		return -1;
+	}
+	(*arg)++;
+
+	return 0;
+}
+
 static size_t parse_rewrap(const char *placeholder, struct rewrap_args *rewrap)
 {
 	unsigned long width = 0, indent1 = 0, indent2 = 0;
@@ -1735,20 +1748,15 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	}
 
 	if (skip_prefix(placeholder, "(decorate", &arg)) {
-		struct decoration_options opts = { NULL };
-		size_t ret = 0;
+		struct decoration_options opts;
 
-		if (*arg == ':') {
-			arg++;
-			parse_decoration_options(&arg, &opts);
-		}
-		if (*arg == ')') {
-			format_decorations(sb, commit, c->auto_color, &opts);
-			ret = arg - placeholder + 1;
-		}
+		if (parse_decoration_options(&arg, &opts) < 0)
+			return 0;
+
+		format_decorations(sb, commit, c->auto_color, &opts);
 
 		free_decoration_options(&opts);
-		return ret;
+		return arg - placeholder;
 	}
 
 	/* For the rest we have to parse the commit header. */
-- 
2.49.0.472.ge94155a9ec