diff mbox series

[2/5] ref-filter: add `short` option for 'tree' and 'parent'

Message ID 49344f1b5559e7b4c63bad323a4dab5956331dde.1595882588.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Improvements to ref-filter | expand

Commit Message

Linus Arver via GitGitGadget July 27, 2020, 8:43 p.m. UTC 
From: Hariom Verma <hariom18599@gmail.com>

Sometimes while using 'parent' and 'tree' atom, user might
want to see abbrev hash instead of full 40 character hash.

'objectname' and 'refname' already supports printing abbrev hash.
It might be convenient for users to have the same option for printing
'parent' and 'tree' hash.

Let's introduce `short` option to 'parent' and 'tree' atom.

`tree:short` - for abbrev tree hash
`parent:short` - for abbrev parent hash

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Heba Waly <heba.waly@gmail.com>
Signed-off-by: Hariom Verma <hariom18599@gmail.com>
---
 ref-filter.c            | 12 ++++++++----
 t/t6300-for-each-ref.sh |  4 ++++
 2 files changed, 12 insertions(+), 4 deletions(-)

Comments

Junio C Hamano July 27, 2020, 11:21 p.m. UTC | #1 
"Hariom Verma via GitGitGadget" <gitgitgadget@gmail.com> writes:

> -		if (!strcmp(name, "tree")) {
> +		if (!strcmp(name, "tree"))
>  			v->s = xstrdup(oid_to_hex(get_commit_tree_oid(commit)));
> -		}
> +		else if (!strcmp(name, "tree:short"))
> +			v->s = xstrdup(find_unique_abbrev(get_commit_tree_oid(commit), DEFAULT_ABBREV));

Again, isn't this going in totally unacceptable direction?  

By the time grab_foo() helper functions are reached, the requested
format should have been parsed to atom->u.foo.option and the only
thing grab_foo() helper functions should look at are the option.

Perhaps studying how "objectname" and its ":"-modified forms are
handled before writing this series would be beneficial.

 - objectname_atom_parser() is called when the parser for --format
   notices "objectname:modifier"; it is responsible for setting up
   atom->u.objectname.option.  Note that this is done only once at
   the very begining of processing

 - grab_objectname() is called for each and every object ref-filter
   iterates over and "objectname" and/or its modified form
   (e.g. "objectname:short") is requested.  Since the modifier is
   already parsed, it can do a simple switch on the value in
   .option.

I do not know if patches [3-5/5] follow the pattern used in [1-2/5],
but if they do, then they all need to be fixed, I think.

Thanks.
diff mbox series

Patch

diff --git a/ref-filter.c b/ref-filter.c
index 8563088eb1..d5d5ff6a9d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -983,21 +983,25 @@  static void grab_commit_values(struct atom_value *val, int deref, struct object
 			continue;
 		if (deref)
 			name++;
-		if (!strcmp(name, "tree")) {
+		if (!strcmp(name, "tree"))
 			v->s = xstrdup(oid_to_hex(get_commit_tree_oid(commit)));
-		}
+		else if (!strcmp(name, "tree:short"))
+			v->s = xstrdup(find_unique_abbrev(get_commit_tree_oid(commit), DEFAULT_ABBREV));
 		else if (!strcmp(name, "numparent")) {
 			v->value = commit_list_count(commit->parents);
 			v->s = xstrfmt("%lu", (unsigned long)v->value);
 		}
-		else if (!strcmp(name, "parent")) {
+		else if (starts_with(name, "parent")) {
 			struct commit_list *parents;
 			struct strbuf s = STRBUF_INIT;
 			for (parents = commit->parents; parents; parents = parents->next) {
 				struct commit *parent = parents->item;
 				if (parents != commit->parents)
 					strbuf_addch(&s, ' ');
-				strbuf_addstr(&s, oid_to_hex(&parent->object.oid));
+				if (!strcmp(name, "parent"))
+					strbuf_addstr(&s, oid_to_hex(&parent->object.oid));
+				else if (!strcmp(name, "parent:short"))
+					strbuf_add_unique_abbrev(&s, &parent->object.oid, DEFAULT_ABBREV);
 			}
 			v->s = strbuf_detach(&s, NULL);
 		}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index b8a2cb8439..533827d297 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -97,7 +97,9 @@  test_atom head objectname:short $(git rev-parse --short refs/heads/master)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
 test_atom head objectname:short=10 $(git rev-parse --short=10 refs/heads/master)
 test_atom head tree $(git rev-parse refs/heads/master^{tree})
+test_atom head tree:short $(git rev-parse --short refs/heads/master^{tree})
 test_atom head parent ''
+test_atom head parent:short ''
 test_atom head numparent 0
 test_atom head object ''
 test_atom head type ''
@@ -148,7 +150,9 @@  test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
 test_atom head objectname:short=10 $(git rev-parse --short=10 refs/heads/master)
 test_atom tag tree ''
+test_atom tag tree:short ''
 test_atom tag parent ''
+test_atom tag parent:short ''
 test_atom tag numparent ''
 test_atom tag object $(git rev-parse refs/tags/testtag^0)
 test_atom tag type 'commit'