Subject: Re: [PATCH v3 1/2] ref-filter: add multiple-option parsing functions

From: Junio C Hamano <[email-masked]>
Date: Thu, 20 Jul 2023 10:59:39 -0700

Kousik Sanagavarapu <[email-masked]> writes:

> Going off in a tangent here---In yesterday's review club which discussed
> the %(decorate:<options>) patch[1], Glen suggested the possibility of
> having a single kind of a framework (used this word very loosely here)
> for parsing these multiple options since we are beginning to see them so
> often (might also help new formats which maybe added in the future). The
> fact that this wasn't done already says something about its difficulty
> as Jacob mentioned yesterday. The difficulty being we don't exactly know
> which options to parse as they differ from format to format.

Yes, while writing the sample in-code documentation for the
match_atom_arg_value() function, I found its interface to force the
callers to do "parse if it is 'tags'; else parse if it is 'abbrev';
and so on" hard to use.  I wondered if the primitive to parse these
should be modeled after config.c:parse_config_key(), i.e. the helper
function takes the input and returns the split point and lengths of
the components it finds in the return parameter.

As the contract between the caller and the callee is that the caller
passes the beginning of "key1[=val1],key2[=val2],...", the interface
may look like

	int parse_cskv(const char *to_parse,
		       const char **key, size_t *keylen,
		       const char **val, size_t *vallen,
		       const char **end);

and would be used like so:

	const char *cskv = "key1,key2=val2";
	const char *key, *val;
	size_t keylen, vallen;

	while (parse_cskv(cskv, &key, &keylen, &val, &vallen, &cskv) {
		if (!val)
			printf("valueless key '%.*s'\n",
			       (int)keylen, key);
		else
			printf("key-value pair '%.*s=%.*s'\n",
			       (int)keylen, key, (int)vallen, val);
	}
	if (*cskv)
		fprintf(stderr, "error - trailing garbage seen '%s'\n",
			cskv);

The helper's contract to the caller may look like this:

 - It expects the string to be "key" or "key=val" followed by ',' or
   '\0' (the latter ends the input, i.e. the last element of comma
   separated list).  The out variables key, val, keylen, vallen are
   used to convey to the caller where key and val are found and how
   long they are.

 - If it is a valueless "key", the out variable val is set to NULL.

 - The out variable end is updated to point at one byte after the
   element that has just been parsed.

The need for the caller to check against the list of keys it knows
about in the loop still exists, but the parser may become simpler
that way.  I dunno.