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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 20, 2023, 17:59 UTC
Message-ID: <xmqq351i4g6s.fsf@gitster.g>
In-Reply-To: <ZLlmXNt2crTEIXLg@five231003>

Kousik Sanagavarapu <five231003@gmail.com> writes:

Show 8 quoted lines

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