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

Hi JAYATHEERTH,

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. (´～`)

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