git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 3/4] pretty.c: refactor trailer logic to `format_set_trailers_options()`

From
HGHariom Verma via GitGitGadget <gitgitgadget@gmail.com>
Date
Aug 21, 2020, 21:06 UTC
Message-ID
<712ab9aacf240a02d808af6b6837e682b929493c.1598043976.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.707.v3.git.1598043976.gitgitgadget@gmail.com>
From: Hariom Verma <hariom18599@gmail.com>

Refactored trailers formatting logic inside pretty.c to a new function `format_set_trailers_options()`.

Also, introduced a code to get invalid trailer arguments. As we would like to use same logic in ref-filter, it's nice to get invalid trailer argument. This will allow us to print accurate error message, while using `format_set_trailers_options()` in ref-filter.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Heba Waly <heba.waly@gmail.com>
Signed-off-by: Hariom Verma <hariom18599@gmail.com>
---
 Hariom Verma via GitGitGadget |  0
 pretty.c                      | 83 ++++++++++++++++++++++-------------
 pretty.h                      | 11 +++++
 3 files changed, 64 insertions(+), 30 deletions(-)
 create mode 100644 Hariom Verma via GitGitGadget
diff --git a/Hariom Verma via GitGitGadget b/Hariom Verma via GitGitGadget
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/pretty.c b/pretty.c
index 2a3d46bf42..bd8d38e27b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1147,6 +1147,55 @@ static int format_trailer_match_cb(const struct strbuf *key, void *ud)
 	return 0;
 }
 
+int format_set_trailers_options(struct process_trailer_options *opts,
+				struct string_list *filter_list,
+				struct strbuf *sepbuf,
+				const char **arg,
+				const char **err)
+{
+	for (;;) {
+		const char *argval;
+		size_t arglen;
+
+		if (**arg != ')') {
+			size_t vallen = strcspn(*arg, "=,)");
+			const char *valstart = xstrndup(*arg, vallen);
+			if (strcmp(valstart, "key") &&
+				strcmp(valstart, "separator") &&
+				strcmp(valstart, "only") &&
+				strcmp(valstart, "valueonly") &&
+				strcmp(valstart, "unfold")) {
+					*err = xstrdup(valstart);
+					return 1;
+				}
+			free((char *)valstart);
+		}
+		if (match_placeholder_arg_value(*arg, "key", arg, &argval, &arglen)) {
+			uintptr_t len = arglen;
+
+			if (!argval)
+				return 1;
+
+			if (len && argval[len - 1] == ':')
+				len--;
+			string_list_append(filter_list, argval)->util = (char *)len;
+
+			opts->filter = format_trailer_match_cb;
+			opts->filter_data = filter_list;
+			opts->only_trailers = 1;
+		} else if (match_placeholder_arg_value(*arg, "separator", arg, &argval, &arglen)) {
+			char *fmt = xstrndup(argval, arglen);
+			strbuf_expand(sepbuf, fmt, strbuf_expand_literal_cb, NULL);
+			free(fmt);
+			opts->separator = sepbuf;
+		} else if (!match_placeholder_bool_arg(*arg, "only", arg, &opts->only_trailers) &&
+				!match_placeholder_bool_arg(*arg, "unfold", arg, &opts->unfold) &&
+				!match_placeholder_bool_arg(*arg, "valueonly", arg, &opts->value_only))
+			break;
+	}
+	return 0;
+}
+
 static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 				const char *placeholder,
 				void *context)
@@ -1417,41 +1466,14 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 		struct string_list filter_list = STRING_LIST_INIT_NODUP;
 		struct strbuf sepbuf = STRBUF_INIT;
 		size_t ret = 0;
+		const char *unused = NULL;
 
 		opts.no_divider = 1;
 
 		if (*arg == ':') {
 			arg++;
-			for (;;) {
-				const char *argval;
-				size_t arglen;
-
-				if (match_placeholder_arg_value(arg, "key", &arg, &argval, &arglen)) {
-					uintptr_t len = arglen;
-
-					if (!argval)
-						goto trailer_out;
-
-					if (len && argval[len - 1] == ':')
-						len--;
-					string_list_append(&filter_list, argval)->util = (char *)len;
-
-					opts.filter = format_trailer_match_cb;
-					opts.filter_data = &filter_list;
-					opts.only_trailers = 1;
-				} else if (match_placeholder_arg_value(arg, "separator", &arg, &argval, &arglen)) {
-					char *fmt;
-
-					strbuf_reset(&sepbuf);
-					fmt = xstrndup(argval, arglen);
-					strbuf_expand(&sepbuf, fmt, strbuf_expand_literal_cb, NULL);
-					free(fmt);
-					opts.separator = &sepbuf;
-				} else if (!match_placeholder_bool_arg(arg, "only", &arg, &opts.only_trailers) &&
-					   !match_placeholder_bool_arg(arg, "unfold", &arg, &opts.unfold) &&
-					   !match_placeholder_bool_arg(arg, "valueonly", &arg, &opts.value_only))
-					break;
-			}
+			if (format_set_trailers_options(&opts, &filter_list, &sepbuf, &arg, &unused))
+				goto trailer_out;
 		}
 		if (*arg == ')') {
 			format_trailers_from_commit(sb, msg + c->subject_off, &opts);
@@ -1460,6 +1482,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 	trailer_out:
 		string_list_clear(&filter_list, 0);
 		strbuf_release(&sepbuf);
+		free((char *)unused);
 		return ret;
 	}
 
diff --git a/pretty.h b/pretty.h
index 071f2fb8e4..cfe2e8b39b 100644
--- a/pretty.h
+++ b/pretty.h
@@ -6,6 +6,7 @@
 
 struct commit;
 struct strbuf;
+struct process_trailer_options;
 
 /* Commit formats */
 enum cmit_fmt {
@@ -139,4 +140,14 @@ const char *format_subject(struct strbuf *sb, const char *msg,
 /* Check if "cmit_fmt" will produce an empty output. */
 int commit_format_is_empty(enum cmit_fmt);
 
+/*
+ * Set values of fields in "struct process_trailer_options"
+ * according to trailers arguments.
+ */
+int format_set_trailers_options(struct process_trailer_options *opts,
+				struct string_list *filter_list,
+				struct strbuf *sepbuf,
+				const char **arg,
+				const char **err);
+
 #endif /* PRETTY_H */
-- 
gitgitgadget
Previous: Hariom Verma via GitGitGadgetNext: Junio C Hamano
Message 29 of 31 in “Fix trailers atom bug and improved tests”
  1. 0/2 Fix trailers atom bug and improved testsHariom Verma via GitGitGadget, Aug 19, 2020
  2. 1/2 t6300: unify %(trailers) and %(contents:trailers) testsHariom Verma via GitGitGadget, Aug 19, 2020
  3. Junio C HamanoAug 19, 2020
  4. Hariom vermaAug 21, 2020
  5. 2/2 ref-filter: 'contents:trailers' show error if `:` is missingHariom Verma via GitGitGadget, Aug 19, 2020
  6. Junio C HamanoAug 19, 2020
  7. Junio C HamanoAug 19, 2020
  8. Eric SunshineAug 19, 2020
  9. Junio C HamanoAug 19, 2020
  10. Hariom vermaAug 20, 2020
  11. 0/2 Fix trailers atom bug and improved testsHariom Verma via GitGitGadget, Aug 21, 2020
  12. 1/2 t6300: unify %(trailers) and %(contents:trailers) testsHariom Verma via GitGitGadget, Aug 21, 2020
  13. 2/2 ref-filter: 'contents:trailers' show error if `:` is missingHariom Verma via GitGitGadget, Aug 21, 2020
  14. Eric SunshineAug 21, 2020
  15. Junio C HamanoAug 21, 2020
  16. Hariom vermaAug 23, 2020
  17. Eric SunshineAug 24, 2020
  18. Hariom vermaAug 24, 2020
  19. Christian CouderAug 26, 2020
  20. Christian CouderAug 26, 2020
  21. Hariom vermaAug 26, 2020
  22. 0/4 [GSoC] Fix trailers atom bug and improved testsHariom Verma via GitGitGadget, Aug 21, 2020
  23. 1/4 t6300: unify %(trailers) and %(contents:trailers) testsHariom Verma via GitGitGadget, Aug 21, 2020
  24. 2/4 ref-filter: 'contents:trailers' show error if `:` is missingHariom Verma via GitGitGadget, Aug 21, 2020
  25. Eric SunshineAug 21, 2020
  26. Hariom vermaAug 21, 2020
  27. Junio C HamanoAug 21, 2020
  28. 4/4 ref-filter: using pretty.c logic for trailersHariom Verma via GitGitGadget, Aug 21, 2020
  29. 3/4 pretty.c: refactor trailer logic to `format_set_trailers_options()`Hariom Verma via GitGitGadget, Aug 21, 2020
  30. Junio C HamanoAug 21, 2020
  31. Hariom vermaAug 22, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.