ref-filter: add %(raw:textconv) and %(raw:filters)

In order to let git cat-file --batch reuse ref-filter logic, This patch, %(rest), %(raw:textconv), %(raw:filters) atoms and --rest=<rest> option are added to ref-filter.

* %(rest) int the format will be replaced by the <rest> in --rest=<rest>.
* the <rest> in --rest=<rest> can also be used as the <path> for %(raw:textconv) and %(raw:filters).
* %(raw:textconv) can show the object's contents as transformed by a textconv filter.
* %(raw:filters) can show the content as converted by the filters configured in the current working tree for the given <path> (i.e. smudge filters, end-of-line conversion, etc).

The current series is based on 0efed9435 ([GSOC] ref-filter: add %(raw) atom)
https://lore.kernel.org/git/[email protected]/
If necessary, "%(rest)" part can be an independent patch later.

ZheNing Hu (6):
[GSOC] ref-filter: add obj-type check in grab contents [GSOC] ref-filter: add %(raw) atom
[GSOC] ref-filter: use non-const ref_format in *_atom_parser() [GSOC] ref-filter: add %(rest) atom and --rest option [GSOC] ref-filter: teach grab_sub_body_contents() return value and err [GSOC] ref-filter: add %(raw:textconv) and %(raw:filters)

 Documentation/git-branch.txt       |   6 +-
 Documentation/git-for-each-ref.txt |  29 ++-
 Documentation/git-tag.txt          |   4 +
 Documentation/git-verify-tag.txt   |   6 +-
 builtin/branch.c                   |   5 +
 builtin/for-each-ref.c             |  17 +-
 builtin/tag.c                      |   8 +-
 builtin/verify-tag.c               |   1 +
 ref-filter.c                       | 296 ++++++++++++++++++++++-------
 ref-filter.h                       |   9 +-
 t/t3203-branch-output.sh           |  14 ++
 t/t6300-for-each-ref.sh            | 294 ++++++++++++++++++++++++++++
 t/t7004-tag.sh                     |  10 +
 t/t7030-verify-tag.sh              |   8 +
 14 files changed, 633 insertions(+), 74 deletions(-)

base-commit: 1197f1a46360d3ae96bd9c15908a3a6f8e562207
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-972%2Fadlternative%2Fref-filter-texconv-filters-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-972/adlternative/ref-filter-texconv-filters-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/972

gitgitgadget

From: ZheNing Hu <[email protected]>

Only tag and commit objects use grab_sub_body_contents() to grab object contents in the current codebase. We want to teach the function to also handle blobs and trees to get their raw data, without parsing a blob (whose contents looks like a commit or a tag) incorrectly as a commit or a tag.

Skip the block of code that is specific to handling commits and tags early when the given object is of a wrong type to help later addition to handle other types of objects in this function.

---
 ref-filter.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

index 4db0e40ff4c6..5cee6512fbaf 100644
@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size
 }
 
 /* See grab_values */
-static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
+static void grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data)
 {
 	int i;
 	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
 	size_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
+	void *buf = data->content;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct used_atom *atom = &used_atom[i];

@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
 			continue;
 		if (deref)
 			name++;
-		if (strcmp(name, "body") &&
-		    !starts_with(name, "subject") &&
-		    !starts_with(name, "trailers") &&
-		    !starts_with(name, "contents"))
+
+		if ((data->type != OBJ_TAG &&
+		     data->type != OBJ_COMMIT) ||
+		    (strcmp(name, "body") &&
+		     !starts_with(name, "subject") &&
+		     !starts_with(name, "trailers") &&
+		     !starts_with(name, "contents")))
 			continue;
 		if (!subpos)
 			find_subpos(buf,

@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)
  * pointed at by the ref itself; otherwise it is the object the
  * ref (which is a tag) refers to.
  */
-static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf)
+static void grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data)
 {
+	void *buf = data->content;
+
 	switch (obj->type) {
 	case OBJ_TAG:
 		grab_tag_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf);
+		grab_sub_body_contents(val, deref, data);
 		grab_person("tagger", val, deref, buf);
 		break;
 	case OBJ_COMMIT:
 		grab_commit_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf);
+		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
 		break;

@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
 					       oid_to_hex(&oi->oid), ref->refname);
 		}
-		grab_values(ref->value, deref, *obj, oi->content);
+		grab_values(ref->value, deref, *obj, oi);
 	}
 
 	grab_common_values(ref->value, deref, oi);

gitgitgadget

From: ZheNing Hu <[email protected]>

Add new formatting option %(raw), which will print the raw object data without any changes. It will help further to migrate all cat-file formatting logic from cat-file to ref-filter.

The raw data of blob, tree objects may contain '\0', but most of the logic in ref-filter depands on the output of the atom being text (specifically, no embedded NULs in it).

E.g. quote_formatting() use strbuf_addstr() or *._quote_buf() add the data to the buffer. The raw data of a tree object is 100644 one\0..., only the 100644 one will be added to the buffer, which is incorrect.

Therefore, add a new member in struct atom_value: s_size, which can record raw object size, it can help us add raw object data to the buffer or compare two buffers which contain raw object data.

Beyond, --format=%(raw) cannot be used with --python, --shell, --tcl, --perl because if our binary raw data is passed to a variable in the host language, the host language may not support arbitrary binary data in the variables of its string type.

---
 Documentation/git-for-each-ref.txt |   9 ++
 ref-filter.c                       | 140 +++++++++++++++----
 t/t6300-for-each-ref.sh            | 207 +++++++++++++++++++++++++++++
 3 files changed, 328 insertions(+), 28 deletions(-)

index 2ae2478de706..8f8d8cd1e04f 100644
@@ -235,6 +235,15 @@ and `date` to extract the named component.  For email fields (`authoremail`,
 without angle brackets, and `:localpart` to get the part before the `@` symbol
 out of the trimmed email.
 
+The raw data in a object is `raw`.
+
+raw:size::
+	The raw data size of the object.
+
+Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,
+`--perl` because the host language may not support arbitrary binary data in the
+variables of its string type.
+
 The message in a commit or a tag object is `contents`, from which
 `contents:<part>` can be used to extract various parts out of:
 

gitgitgadget

From: ZheNing Hu <[email protected]>

Use non-const ref_format in *_atom_parser(), which can help us modify the members of ref_format in *_atom_parser().

---
 builtin/tag.c |  2 +-
 ref-filter.c  | 44 ++++++++++++++++++++++----------------------
 ref-filter.h  |  4 ++--
 3 files changed, 25 insertions(+), 25 deletions(-)

index 82fcfc098242..452558ec9575 100644
@@ -146,7 +146,7 @@ static int verify_tag(const char *name, const char *ref,
 		      const struct object_id *oid, void *cb_data)
 {
 	int flags;
-	const struct ref_format *format = cb_data;
+	struct ref_format *format = cb_data;
 	flags = GPG_VERIFY_VERBOSE;
 
 	if (format->format)

index baf72a718965..74fb423fc89f 100644
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
 void ref_sorting_set_sort_flags_all(struct ref_sorting *sorting, unsigned int mask, int on);
 /*  Based on the given format and quote_style, fill the strbuf */
 int format_ref_array_item(struct ref_array_item *info,
-			  const struct ref_format *format,
+			  struct ref_format *format,
 			  struct strbuf *final_buf,
 			  struct strbuf *error_buf);
 /*  Parse a single sort specifier and add it to the list */

@@ -137,7 +137,7 @@ void setup_ref_filter_porcelain_msg(void);
  * name must be a fully qualified refname.
  */
 void pretty_print_ref(const char *name, const struct object_id *oid,
-		      const struct ref_format *format);
+		      struct ref_format *format);
 
 /*
  * Push a single ref onto the array; this can be used to construct your own

gitgitgadget

From: ZheNing Hu <[email protected]>

In order to let "cat-file --batch=%(rest)" use the ref-filter interface, add %(rest) atom for ref-filter and --rest option for "git for-each-ref", "git branch", "git tag" and "git verify-tag". --rest specify a string to replace %(rest) placeholders of the --format option.

---
 Documentation/git-branch.txt       |  6 +++++-
 Documentation/git-for-each-ref.txt | 10 +++++++++-
 Documentation/git-tag.txt          |  4 ++++
 Documentation/git-verify-tag.txt   |  6 +++++-
 builtin/branch.c                   |  5 +++++
 builtin/for-each-ref.c             |  6 ++++++
 builtin/tag.c                      |  6 ++++++
 builtin/verify-tag.c               |  1 +
 ref-filter.c                       | 21 +++++++++++++++++++++
 ref-filter.h                       |  5 ++++-
 t/t3203-branch-output.sh           | 14 ++++++++++++++
 t/t6300-for-each-ref.sh            | 24 ++++++++++++++++++++++++
 t/t7004-tag.sh                     | 10 ++++++++++
 t/t7030-verify-tag.sh              |  8 ++++++++
 14 files changed, 122 insertions(+), 4 deletions(-)

index 94dc9a54f2d7..29fa579e3d8a 100644
@@ -15,7 +15,7 @@ SYNOPSIS
 	[--contains [<commit>]] [--no-contains [<commit>]]
 	[--points-at <object>] [--format=<format>]
 	[(-r | --remotes) | (-a | --all)]
-	[--list] [<pattern>...]
+	[--list] [<pattern>...] [--rest=<rest>]
 'git branch' [--track | --no-track] [-f] <branchname> [<start-point>]
 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
 'git branch' --unset-upstream [<branchname>]

@@ -298,6 +298,10 @@ start-point is either a local or remote-tracking branch.
 	and the object it points at.  The format is the same as
 	that of linkgit:git-for-each-ref[1].
 
+--rest=<rest>::
+	If given, the `%(rest)` placeholders in the `--format` option
+	will be replaced.
+
 CONFIGURATION
 -------------
 `pager.branch` is only respected when listing branches, i.e., when

index 8f8d8cd1e04f..e85b7b19a530 100644
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>] [<pattern>...]
-		   [--points-at=<object>]
+		   [--points-at=<object>] [--rest=<rest>]
 		   [--merged[=<object>]] [--no-merged[=<object>]]
 		   [--contains[=<object>]] [--no-contains[=<object>]]
 

@@ -74,6 +74,10 @@ OPTIONS
 --points-at=<object>::
 	Only list refs which points at the given object.
 
+--rest=<rest>::
+	If given, the `%(rest)` placeholders in the `--format` option
+	will be replaced by its contents.
+
 --merged[=<object>]::
 	Only list refs whose tips are reachable from the
 	specified commit (HEAD if not specified).

@@ -235,6 +239,10 @@ and `date` to extract the named component.  For email fields (`authoremail`,
 without angle brackets, and `:localpart` to get the part before the `@` symbol
 out of the trimmed email.
 
+rest::
+	The placeholder for `--rest` specified string. If no `--rest`, nothing
+	is printed.
+
 The raw data in a object is `raw`.
 
 raw:size::

index 31a97a1b6c5b..3bf6ae7216de 100644
@@ -200,6 +200,10 @@ This option is only applicable when listing tags without annotation lines.
 	that of linkgit:git-for-each-ref[1].  When unspecified,
 	defaults to `%(refname:strip=2)`.
 
+--rest=<rest>::
+	If given, the `%(rest)` placeholders in the `--format` option
+	will be replaced.
+
 <tagname>::
 	The name of the tag to create, delete, or describe.
 	The new tag name must pass all checks defined by

index 0b8075dad965..7ba4a6941ab1 100644
@@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags
 SYNOPSIS
 --------
 [verse]
-'git verify-tag' [--format=<format>] <tag>...
+'git verify-tag' [--format=<format>] [--rest=<rest>] <tag>...
 
 DESCRIPTION
 -----------

@@ -27,6 +27,10 @@ OPTIONS
 <tag>...::
 	SHA-1 identifiers of Git tag objects.
 
+--rest=<rest>::
+	If given, the `%(rest)` placeholders in the `--format` option
+	will be replaced.
+
 GIT
 ---
 Part of the linkgit:git[1] suite

index b23b1d1752af..b03a4c49c1d9 100644
@@ -439,6 +439,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 	if (verify_ref_format(format))
 		die(_("unable to parse format string"));
 
+	if (format->use_rest)
+		for (i = 0; i < array.nr; i++)
+			array.items[i]->rest = format->rest;
+
 	ref_array_sort(sorting, &array);
 
 	for (i = 0; i < array.nr; i++) {

@@ -670,6 +674,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			N_("print only branches of the object"), parse_opt_object_name),
 		OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_STRING(  0 , "format", &format.format, N_("format"), N_("format to use for the output")),
+		OPT_STRING(0, "rest", &format.rest, N_("rest"), N_("specify %(rest) contents")),
 		OPT_END(),
 	};
 

index 89cb6307d46f..fac7777fd2c0 100644
@@ -37,6 +37,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
+		OPT_STRING(  0 , "rest", &format.rest, N_("rest"), N_("specify %(rest) contents")),
 		OPT_STRING(  0 , "format", &format.format, N_("format"), N_("format to use for the output")),
 		OPT__COLOR(&format.use_color, N_("respect format colors")),
 		OPT_REF_SORT(sorting_tail),

@@ -78,6 +79,11 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	filter.name_patterns = argv;
 	filter.match_as_path = 1;
 	filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);
+
+	if (format.use_rest)
+		for (i = 0; i < array.nr; i++)
+			array.items[i]->rest = format.rest;
+
 	ref_array_sort(sorting, &array);
 
 	if (!maxcount || array.nr < maxcount)

index 452558ec9575..9e52b3eacb16 100644
@@ -63,6 +63,11 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting,
 		die(_("unable to parse format string"));
 	filter->with_commit_tag_algo = 1;
 	filter_refs(&array, filter, FILTER_REFS_TAGS);
+
+	if (format->use_rest)
+		for (i = 0; i < array.nr; i++)
+			array.items[i]->rest = format->rest;
+
 	ref_array_sort(sorting, &array);
 
 	for (i = 0; i < array.nr; i++) {

@@ -481,6 +486,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_STRING(  0 , "format", &format.format, N_("format"),
 			   N_("format to use for the output")),
 		OPT__COLOR(&format.use_color, N_("respect format colors")),
+		OPT_STRING(0, "rest", &format.rest, N_("rest"), N_("specify %(rest) contents")),
 		OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END()
 	};

index f45136a06ba7..00be4899678d 100644
@@ -36,6 +36,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 		OPT__VERBOSE(&verbose, N_("print tag contents")),
 		OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW),
 		OPT_STRING(0, "format", &format.format, N_("format"), N_("format to use for the output")),
+		OPT_STRING(0, "rest", &format.rest, N_("rest"), N_("specify %(rest) contents")),
 		OPT_END()
 	};
 

index 608e38aa4160..695f6f55e3e3 100644
@@ -157,6 +157,7 @@ enum atom_type {
 	ATOM_IF,
 	ATOM_THEN,
 	ATOM_ELSE,
+	ATOM_REST,
 };
 
 /*

@@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,
 	return 0;
 }
 
+static int rest_atom_parser(struct ref_format *format, struct used_atom *atom,
+			    const char *arg, struct strbuf *err)
+{
+	if (arg)
+		return strbuf_addf_ret(err, -1, _("%%(rest) does not take arguments"));
+	format->use_rest = 1;
+	return 0;
+}
+
 static int head_atom_parser(struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *unused_err)
 {

@@ -615,6 +625,7 @@ static struct {
 	[ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser },
 	[ATOM_THEN] = { "then", SOURCE_NONE },
 	[ATOM_ELSE] = { "else", SOURCE_NONE },
+	[ATOM_REST] = { "rest", SOURCE_NONE, FIELD_STR, rest_atom_parser },
 	/*
 	 * Please update $__git_ref_fieldlist in git-completion.bash
 	 * when you add new atoms

@@ -1919,6 +1930,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			v->handler = else_atom_handler;
 			v->s = xstrdup("");
 			continue;
+		} else if (atom_type == ATOM_REST) {
+			if (ref->rest)
+				v->s = xstrdup(ref->rest);
+			else
+				v->s = xstrdup("");
+			continue;
 		} else
 			continue;
 

@@ -2136,6 +2153,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
 
 	FLEX_ALLOC_STR(ref, refname, refname);
 	oidcpy(&ref->objectname, oid);
+	ref->rest = NULL;
 
 	return ref;
 }

@@ -2600,6 +2618,9 @@ void pretty_print_ref(const char *name, const struct object_id *oid,
 
 	ref_item = new_ref_array_item(name, oid);
 	ref_item->kind = ref_kind_from_refname(name);
+	if (format->use_rest)
+		ref_item->rest = format->rest;
+
 	if (format_ref_array_item(ref_item, format, &output, &err))
 		die("%s", err.buf);
 	fwrite(output.buf, 1, output.len, stdout);

index 74fb423fc89f..9dc07476a584 100644
@@ -38,6 +38,7 @@ struct ref_sorting {
 
 struct ref_array_item {
 	struct object_id objectname;
+	const char *rest;
 	int flag;
 	unsigned int kind;
 	const char *symref;

@@ -76,14 +77,16 @@ struct ref_format {
 	 * verify_ref_format() afterwards to finalize.
 	 */
 	const char *format;
+	const char *rest;
 	int quote_style;
+	int use_rest;
 	int use_color;
 
 	/* Internal state to ref-filter */
 	int need_color_reset_at_eol;
 };
 
-#define REF_FORMAT_INIT { NULL, 0, -1 }
+#define REF_FORMAT_INIT { NULL, NULL, 0, 0, -1 }
 
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \

index 5325b9f67a00..dc6bf2557088 100755
@@ -340,6 +340,20 @@ test_expect_success 'git branch --format option' '
 	test_cmp expect actual
 '
 
+test_expect_success 'git branch with --format and --rest option' '
+	cat >expect <<-\EOF &&
+	@Refname is (HEAD detached from fromtag)
+	@Refname is refs/heads/ambiguous
+	@Refname is refs/heads/branch-one
+	@Refname is refs/heads/branch-two
+	@Refname is refs/heads/main
+	@Refname is refs/heads/ref-to-branch
+	@Refname is refs/heads/ref-to-remote
+	EOF
+	git branch --rest="@" --format="%(rest)Refname is %(refname)" >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'worktree colors correct' '
 	cat >expect <<-EOF &&
 	* <GREEN>(HEAD detached from fromtag)<RESET>

index 5f66d933ace0..e908b2ca0522 100755
@@ -1187,6 +1187,30 @@ test_expect_success 'basic atom: head contents:trailers' '
 	test_cmp expect actual.clean
 '
 
+test_expect_success 'bacic atom: rest with --rest' '
+	git for-each-ref --format="###refname=%(refname)
+###oid=%(objectname)
+###type=%(objecttype)
+###size=%(objectsize)" refs/heads/main refs/tags/subject-body >expect &&
+	git for-each-ref --rest="###" --format="%(rest)refname=%(refname)
+%(rest)oid=%(objectname)
+%(rest)type=%(objecttype)
+%(rest)size=%(objectsize)" refs/heads/main refs/tags/subject-body >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'bacic atom: *rest with --rest' '
+	git for-each-ref --format="###refname=%(refname)
+###oid=%(objectname)
+###type=%(objecttype)
+###size=%(objectsize)" refs/heads/main refs/tags/subject-body >expect &&
+	git for-each-ref --rest="###" --format="%(*rest)refname=%(refname)
+%(rest)oid=%(objectname)
+%(rest)type=%(objecttype)
+%(rest)size=%(objectsize)" refs/heads/main refs/tags/subject-body >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'trailer parsing not fooled by --- line' '
 	git commit --allow-empty -F - <<-\EOF &&
 	this is the subject

index 2f72c5c6883e..93c4366ff52d 100755
@@ -1998,6 +1998,16 @@ test_expect_success '--format should list tags as per format given' '
 	test_cmp expect actual
 '
 
+test_expect_success 'tag -l with --format and --rest' '
+	cat >expect <<-\EOF &&
+	#refname : refs/tags/v1.0
+	#refname : refs/tags/v1.0.1
+	#refname : refs/tags/v1.1.3
+	EOF
+	git tag -l --rest="#" --format="%(rest)refname : %(refname)" "v1*" >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success "set up color tests" '
 	echo "<RED>v1.0<RESET>" >expect.color &&
 	echo "v1.0" >expect.bare &&

index 3cefde9602bf..008df00c32d6 100755
@@ -194,6 +194,14 @@ test_expect_success GPG 'verifying tag with --format' '
 	test_cmp expect actual
 '
 
+test_expect_success GPG 'verifying tag with --format and --rest' '
+	cat >expect <<-\EOF &&
+	#tagname : fourth-signed
+	EOF
+	git verify-tag --rest="#" --format="%(rest)tagname : %(tag)" "fourth-signed" >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success GPG 'verifying a forged tag with --format should fail silently' '
 	test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged &&
 	test_must_be_empty actual-forged

gitgitgadget

Hi,

Hi,

Yeah, I noticed it too.

Maybe... I find it's begin at c3428da8 (Make builtin-for-each-ref.c use parse-opts.)
This is something from 2007. So It may be wrong to imitate its format. But this format fix work may be can put in good-first-issue. :)

Thanks.
--
ZheNing Hu

I cannot think of a sane reason why we need to allow "%(rest)" in anithing but "cat-file --batch", where a natural source of %(rest) exists in its input stream (i.e. each input record begins with an object name to be processed, and the rest of the record can become "%(rest)").

The "cat-file --batch" thing is much more understandable. You could for example:

    git ls-files -s |
    sed -e 's/^[0-7]* \([0-9a-f]*\) [0-3]	/\1 /' |
    git cat-file --batch='%(objectname) %(objecttype) %(rest)'

to massage output from "ls-files -s" like this

    100644 c2f5fe385af1bbc161f6c010bdcf0048ab6671ed 0	.cirrus.yml
    100644 c592dda681fecfaa6bf64fb3f539eafaf4123ed8 0	.clang-format
    100644 f9d819623d832113014dd5d5366e8ee44ac9666a 0	.editorconfig
    ...

into recods of "<objectname> <path>", and each output record will replay the <path> part from each corresponding input record.

Unless for-each-ref family of commands read the list of refs that it shows from their standard input (they do not, and I do not think it makes any sense to teach them to), there is no place to feed the "rest" information that is associated with each output record. The only thing the commands taught about %(rest) by this patch can do is to parrot the same string into each and every output record. I am not seeing what this new feature is attempting to give us.

If anything, I would imagine that it would be a very useful addition to teach the ref-filter machinery an ability to optionally error out depending on the caller when the caller attempts to use certain placeholder. Then, we can reject "git branch --sort=rest" sensibly, instead of accepting "git branch --sort=rest --rest=constant", which is not technically wrong per-se, but smells like a total nonsense from practical usefulness's point of view.

First of all, although %(rest) is meaningless in ordinary circumstances, ref-filter must learn %(rest), it is impossible for us to leave the parsing of %(rest) in cat-file.c alone.

Then, --rest is a strategy that make %(rest) can use in git for-each-ref or git branch -l. As you said, it is just a boring placeholder used for string replacement. We can make it output only empty content, If we really don’t need --rest.

s/[0-3] /[0-3]\t/

Yeah, the <path> in the input will be treated as "rest".

"parrot the same string"? I think we should use an empty string here, "parrot the same string" more like what the "git log --format" family does.

This sounds like it might help cat-file to reject some useless atoms like %(refname). So something like:

$ git for-each-ref --format="%(objectname) %(objectsize)"
--refject-atoms="%(objectsize) %(objectname)"

will fail.

"git for-each-ref" family use hardcoded to reject %(rest). I can try to achieve this function.

Will output an empty string.

Hope we can reach an agreement:
delete --rest and add --reject-atoms. ;-)

Thanks.
--
ZheNing Hu

I forget one thing: %(raw:textconv) and %(raw:filters) can use the value of "--rest" as their <path>. But now if we want delete --rest, they can not be used for "for-each-ref" family, Git will die with "missing path for 'xxx'".

If we actually delete "--rest", we will have no way to test %(raw:textconv) and %(raw:filters)... So now I think we can keep --rest (or use another name --path)
and let "git for-each-ref" family reject %(rest) by default.

Thanks.
--
ZheNing Hu

I didn't read beyond the %(rest) thing, but do we even need %(raw:textconv) to begin with? It is totally useless in the context of for-each-ref because textconv by its nature is tied to attributes that by definition needs a blob that is sitting at a path, but the objects for-each-ref and friends visit are mostly commits and tags, and even for refs that point at a blob, there isn't any "path" information to pull attribute for.

Is that what you want to add to give "cat-file --batch"? Even in the context of "cat-file --batch", you can throw an object name for a blob to the command, but there is no path for the blob (a blob can appear at different places in different trees---think "rename), so I am not sure what benefit you are trying to derive from it.

Thanks.

After thinking about your words, now I think maybe we can leave %(raw:textconv) and %(raw:filter) after cat-file --batch start using ref-filter logic, so that we can provide them with suitable tests, and we don't need --rest anymore.

So I will remove the last two commits.

Thanks.
--
ZheNing Hu

I think I kind-of see what is going on here. There is

    git cat-file blob --textconv --path="$path" "$blob_object_name"

that allows a blob to be fed to the command, pretend as if it appears at $path in a tree object and grab attribute for it, and show the blob contents converted using the textconv filter. If we were to mimic it by extending the format based substitutions, a design consistent with the behaviour is to teach --format=%(raw) to show the contents after applying the textconv filter instead of the raw blob contents.

And there is a corresponding

    git cat-file --batch --textconv

The "--path=$path" parameter is omitted when using --batch, as each object would sit at different path in the tree (so the input stream would be given as a run of "<blob> <path>" to give each item its own path).

So to answer my question in the previous message, yes, this is an attempt to support the "cat-file --textconv". So in the context of that command, something may need to be added. But I do not think it makes any sense to expose that to for-each-ref and friends, even if we were to share the internal machinery (after all, sharing of the internal machinery is a mere means to an end that is to make it easier to give the same syntax and same behaviour to end users and is not a goal itself; "because we use the same machinery, the users have to tolerate that irrelevant %(atoms) are accepted by the parser" is not making a good excuse for a sloppy implementation).

Having said all that, I somehow doubt that the "--batch=<format>" was designed to interact sensibly with the "--textconv" option. builtin/cat-file.c::expand_atom() does not know anything at all that the data could be modified from the raw contents of the blob, so --batch="%(contents) %(size)" --textconv, if existed, may show the conveted contents with size of blob before conversion, or something incoherent like that. And if your rewrite using the shared internal machinery results in a more coherent behaviour, that would be excellent. For example, we could imagine that the machinery, when textconv (or filter) is in use, would first grab the blob contents and run the requested conversion, and then work solely on that conveted contents when computing what to fill with %(raw:size) and other blob-related atoms.

Thanks.

Yes, this is exactly what cat-file --textconv does.

Just like let --batch omitted --path, --rest is meaningless for "for-ech-ref".

Because "git cat-file --batch" will only print the contents of the object once, so when implements the function of textconv/filters in ref-filter, we should really consider whether we should let something like "%(raw) %(raw) %(raw) %(raw:size)" all pass the conversion of textconv/filters. If it is my previous %(raw:textconv) or %(raw:filter), they can only print the converted content separately, and we need:

$ git for-ecah-ref --format="%(raw:filters) %(raw:filters)
%(raw:filters) %(raw:filters:size)"

As you said it might be too complicated for the user....

And with your --textconv/--filters, we only need:

$ git for-ecah-ref --format="%(raw) %(raw) %(raw) %(raw:size)" --filters

This will be more concise for users. I will try to build --filters, --textconv for ref-filter . But as stated in the previous reply, it needs to be placed after the transplant of cat-file --batch (Because --path is useless for for-each-ref, and at the same time we need proper testing for --filter/--textconv.)

Thanks, your reply is very reasonable,
--
ZheNing Hu

Oh, there is no question about that.

If there is no need to expose %(rest) to the users who write --format for these two commands, it would be much better to detect attempted use of %(rest) and error out.

Yes.

I don't understand. Why do you even need to add --reject? Why would any user would want to use it with for-each-ref?

Without any end-user input, %(rest) for for-each-ref would not make sense, and %(refname) for cat-file --batch would not make sense, I would imagine, so there is no need to be able to tell --reject=rest to for-each-ref. It is not like giving --no-reject=rest to for-each-ref and make it interpolate to an empty string is a useful feature anyway, so I do not see a need for such an option.

OK.

Yeah... I might have thought it complicated before. We can reject %(rest) in verify_ref_format() easily. It's like we refused --<lang> before. :)

Thanks.
--
ZheNing Hu

From: ZheNing Hu <[email protected]>

Teach grab_sub_body_contents() return value and err can help us report more useful error messages when when add %(raw:textconv) and %(raw:filter) later.

---
 ref-filter.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

index 695f6f55e3e3..a859a94aa8e0 100644
@@ -1403,7 +1403,8 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size
 }
 
 /* See grab_values */
-static void grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data)
+static int grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data,
+				  struct strbuf *err)
 {
 	int i;
 	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;

@@ -1478,6 +1479,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
 
 	}
 	free((void *)sigpos);
+	return 0;
 }
 
 /*

@@ -1501,33 +1503,39 @@ static void fill_missing_values(struct atom_value *val)
  * pointed at by the ref itself; otherwise it is the object the
  * ref (which is a tag) refers to.
  */
-static void grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data)
+static int grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data, struct strbuf *err)
 {
 	void *buf = data->content;
+	int ret = 0;
 
 	switch (obj->type) {
 	case OBJ_TAG:
 		grab_tag_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, data);
+		if ((ret = grab_sub_body_contents(val, deref, data, err)))
+			return ret;
 		grab_person("tagger", val, deref, buf);
 		break;
 	case OBJ_COMMIT:
 		grab_commit_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, data);
+		if ((ret = grab_sub_body_contents(val, deref, data, err)))
+			return ret;
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
-		grab_sub_body_contents(val, deref, data);
+		if ((ret = grab_sub_body_contents(val, deref, data, err)))
+			return ret;
 		break;
 	case OBJ_BLOB:
 		/* grab_blob_values(val, deref, obj, buf, sz); */
-		grab_sub_body_contents(val, deref, data);
+		if ((ret = grab_sub_body_contents(val, deref, data, err)))
+			return ret;
 		break;
 	default:
 		die("Eh?  Object of type %d?", obj->type);
 	}
+	return ret;
 }
 
 static inline char *copy_advance(char *dst, const char *src)

@@ -1725,6 +1733,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 {
 	/* parse_object_buffer() will set eaten to 0 if free() will be needed */
 	int eaten = 1;
+	int ret = 0;
+
 	if (oi->info.contentp) {
 		/* We need to know that to use parse_object_buffer properly */
 		oi->info.sizep = &oi->size;

@@ -1745,13 +1755,13 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
 					       oid_to_hex(&oi->oid), ref->refname);
 		}
-		grab_values(ref->value, deref, *obj, oi);
+		ret = grab_values(ref->value, deref, *obj, oi, err);
 	}
 
 	grab_common_values(ref->value, deref, oi);
 	if (!eaten)
 		free(oi->content);
-	return 0;
+	return ret;
 }
 
 static void populate_worktree_map(struct hashmap *map, struct worktree **worktrees)

@@ -1805,7 +1815,7 @@ static char *get_worktree_path(const struct used_atom *atom, const struct ref_ar
 static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 {
 	struct object *obj;
-	int i;
+	int i, ret = 0;
 	struct object_info empty = OBJECT_INFO_INIT;
 
 	CALLOC_ARRAY(ref->value, used_atom_cnt);

@@ -1961,8 +1971,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 
 
 	oi.oid = ref->objectname;
-	if (get_object(ref, 0, &obj, &oi, err))
-		return -1;
+	if ((ret = get_object(ref, 0, &obj, &oi, err)))
+		return ret;
 
 	/*
 	 * If there is no atom that wants to know about tagged

@@ -1993,9 +2003,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 static int get_ref_atom_value(struct ref_array_item *ref, int atom,
 			      struct atom_value **v, struct strbuf *err)
 {
+	int ret = 0;
+
 	if (!ref->value) {
-		if (populate_value(ref, err))
-			return -1;
+		if ((ret = populate_value(ref, err)))
+			return ret;
 		fill_missing_values(ref->value);
 	}
 	*v = &ref->value[atom];

@@ -2568,6 +2580,7 @@ int format_ref_array_item(struct ref_array_item *info,
 			  struct strbuf *error_buf)
 {
 	const char *cp, *sp, *ep;
+	int ret = 0;
 	struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
 
 	state.quote_style = format->quote_style;

@@ -2581,10 +2594,10 @@ int format_ref_array_item(struct ref_array_item *info,
 		if (cp < sp)
 			append_literal(cp, sp, &state);
 		pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf);
-		if (pos < 0 || get_ref_atom_value(info, pos, &atomv, error_buf) ||
+		if (pos < 0 || ((ret = get_ref_atom_value(info, pos, &atomv, error_buf))) ||
 		    atomv->handler(atomv, &state, error_buf)) {
 			pop_stack_element(&state.stack);
-			return -1;
+			return ret ? ret : -1;
 		}
 	}
 	if (*cp) {

gitgitgadget

From: ZheNing Hu <[email protected]>

In order to let git cat-file --batch --filter and git cat-file --batch --textconv use ref-filter interface, %(raw:textconv) and %(raw:filters) are added to ref-filter.

--rest contents is used as the <path> for them.

%(raw:textconv) can show the object' contents as transformed by a textconv filter.

%(raw:filters) can show the content as converted by the filters configured in the current working tree for the given <path> (i.e. smudge filters, end-of-line conversion, etc).

In addition, they cannot be used with --python, --tcl, --shell, --perl.

---
 Documentation/git-for-each-ref.txt | 18 ++++++--
 builtin/for-each-ref.c             | 11 ++++-
 ref-filter.c                       | 68 ++++++++++++++++++++++++------
 t/t6300-for-each-ref.sh            | 63 +++++++++++++++++++++++++++
 4 files changed, 141 insertions(+), 19 deletions(-)

index e85b7b19a530..e49b8861f474 100644
@@ -76,7 +76,8 @@ OPTIONS
 
 --rest=<rest>::
 	If given, the `%(rest)` placeholders in the `--format` option
-	will be replaced by its contents.
+	will be replaced by its contents. At the same time, its contents
+	is used as the `<path>` for `%(raw:textconv)` and `%(raw:filter)`.
 
 --merged[=<object>]::
 	Only list refs whose tips are reachable from the

@@ -248,9 +249,18 @@ The raw data in a object is `raw`.
 raw:size::
 	The raw data size of the object.
 
-Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,
-`--perl` because the host language may not support arbitrary binary data in the
-variables of its string type.
+raw:textconv::
+	Show the content as transformed by a textconv filter. It must be used
+	with `--rest`.
+
+raw:filters::
+	Show the content as converted by the filters configured in
+	the current working tree for the given `<path>` (i.e. smudge filters,
+	end-of-line conversion, etc). It must be used with `--rest`.
+
+Note that `--format=%(raw)`, `--format=%(raw:textconv)`, `--format=%(raw:filter)`
+can not be used with `--python`, `--shell`, `--tcl`, `--perl` because the host
+language may not support arbitrary binary data in the variables of its string type.
 
 The message in a commit or a tag object is `contents`, from which
 `contents:<part>` can be used to extract various parts out of:

index fac7777fd2c0..4be5ddacbac1 100644
@@ -5,6 +5,7 @@
 #include "object.h"
 #include "parse-options.h"
 #include "ref-filter.h"
+#include "userdiff.h"
 
 static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),

@@ -14,6 +15,14 @@ static char const * const for_each_ref_usage[] = {
 	NULL
 };
 
+static int git_for_each_ref_config(const char *var, const char *value, void *cb)
+{
+	if (userdiff_config(var, value) < 0)
+		return -1;
+
+	return git_default_config(var, value, cb);
+}
+
 int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
 	int i;

@@ -57,7 +66,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 
 	format.format = "%(objectname) %(objecttype)\t%(refname)";
 
-	git_config(git_default_config, NULL);
+	git_config(git_for_each_ref_config, NULL);
 
 	parse_options(argc, argv, prefix, opts, for_each_ref_usage, 0);
 	if (maxcount < 0) {

gitgitgadget

Hi,

Better say "Add ... atoms and --rest=<rest> option to ref-filter, in order to let git cat-file reuse ref-filter logic."

s/int/in/

Did you mean that %(rest) atom can also be used as <path> for the latter?

An old man doll... just what I always wanted! - Clara

OK.

No. just the <rest> in --rest=<rest> will be treated as <path> for %(raw:textconv)
and %(raw:filters).

Thanks.
--
ZheNing Hu

I do not have that commit object, but these six patches include the two commits that are %(raw) and %(raw:size), so I'll just discard the old round that wasn't based on the atom-type stuff and queue these six as a single series.

As I already said, I am not sure how %(rest) makes any sense outside the context of "cat-file --batch"; I suspect it would make more sense to make it easier to arrange certain placeholders to error out when used in a context where they do not make sense (e.g. use of --rest in "git branch --list").

Well, it is a commit that has not been sent to the mailing list. But it’s okay to treat the new 6 commits as a new patch series.

I agree.

--
ZheNing Hu

I haven't gotten around looking at anything after the %(rest) one, but

https://github.com/git/git/runs/2770688471?check_suite_focus=true

seems to tell us that there is "size_t *" vs "ulong *" type confusion, possibly around the textconv thing.

ok, I will change it when we use %(raw:textconv) next time.

Thanks.
--
ZheNing Hu