Re: [PATCH 0/4] repo: add support for path-related fields

[Tian Yuchen <a3205153416@gmail.com>]

Hi JAYATHEERTH,

> I see your point here.
> but wouldn't this effectively be the same as Ayush's suggestion, just
> with a different syntax?

In my view, this issue is actually to choose the most suitable tool for 
the job. After all, we don't want to use something like rev-parse, which 
is riddled with *ancient* technical debt, nor do we want to write an 
even more verbose parsing function from scratch for what you call 
verbose user input, right?

If I'm not mistaken, using different parsing functions to parse input is 
absolutely not just a matter of syntax differences. Instead, this will 
directly result in differences in data structures.

In ref-filter.c, we can easily see how Git parses format modifiers.

After the user input is parse by parse_ref_filter_atom(), it is then 
passed to the atom_valid[] static registry, which is like;

static struct {
	const char *name;
	info_source source;
	cmp_type cmp_type;
	int (*parser)(struct ref_format *format, struct used_atom *atom,
		      const char *arg, struct strbuf *err);
} valid_atom[] = {
	[ATOM_REFNAME] = { "refname", SOURCE_NONE, FIELD_STR, 
refname_atom_parser },
	[ATOM_OBJECTTYPE] = { "objecttype", SOURCE_OTHER, FIELD_STR, 
objecttype_atom_parser },
	[ATOM_OBJECTSIZE] = { "objectsize", SOURCE_OTHER, FIELD_ULONG, 
objectsize_atom_parser },
	[ATOM_OBJECTNAME] = { "objectname", SOURCE_OTHER, FIELD_STR, 
oid_atom_parser },
...

As you can see, each mapping relationship points to a parsing function 
(..._parser()). This parsing function is solely responsible for handling 
the state arg, fundamentally resolving the issue of function 
responsibility/naming confusion.

The reason I recommend this approach is because its implementation is 
incredibly clear and concise. To achieve the functionality we desire, 
all we need to do is add the following to the registry:

[ATOM_PATH] = { "path", SOURCE_NONE, FIELD_STR, path_atom_parser }

And the corresponding path_atom_parser().

This approach also offers strong scalability: If one day I decide to add 
a new feature like %(path:commondir,relative) output, all it would take 
is adding a switch statement in the parser() function (along with a few 
other minor tweaks).

*I'm not saying this approach is better than the solution you've 
discussed. I'm simply presenting a possible implementation for 
reference. (´～`)

> Coming to user friendliness
> I believe Junio has already raised an appropriate question.

This isn't a case of “you can't have your cake and eat it too,” right? I 
think user-friendliness can be achieved without compromising 
maintainability, predictability, or high performance in this case.

Regards,

Yuchen