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

[Junio C Hamano <gitster@pobox.com>]

"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.