Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Kousik Sanagavarapu <five231003@gmail.com>
Cc: git@vger.kernel.org,
	Christian Couder <christian.couder@gmail.com>,
	Hariom Verma <hariom18599@gmail.com>,
	Josh Steadmon <steadmon@google.com>,
	Siddharth Singh <siddhartth@google.com>,
	Glen Choo <chooglen@google.com>
Subject: Re: [PATCH v3 1/2] ref-filter: add multiple-option parsing functions
Date: Thu, 20 Jul 2023 10:59:39 -0700	[thread overview]
Message-ID: <xmqq351i4g6s.fsf@gitster.g> (raw)
In-Reply-To: <ZLlmXNt2crTEIXLg@five231003> (Kousik Sanagavarapu's message of "Thu, 20 Jul 2023 22:22:44 +0530")

Kousik Sanagavarapu <five231003@gmail.com> 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.


  reply	other threads:[~2023-07-20 17:59 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-05 17:57 [PATCH 0/2] Add new "describe" atom Kousik Sanagavarapu
2023-07-05 17:57 ` [PATCH 1/2] ref-filter: add " Kousik Sanagavarapu
2023-07-06 16:58   ` Junio C Hamano
2023-07-09  6:16     ` Kousik Sanagavarapu
2023-07-05 17:57 ` [PATCH 2/2] t6300: run describe atom tests on a different repo Kousik Sanagavarapu
2023-07-14 19:20 ` [PATCH v2 0/3] Add new "describe" atom Kousik Sanagavarapu
2023-07-14 19:20   ` [RFC PATCH v2 1/3] ref filter: add multiple-option parsing functions Kousik Sanagavarapu
2023-07-14 19:20   ` [PATCH v2 2/3] ref-filter: add new "describe" atom Kousik Sanagavarapu
2023-07-14 20:57     ` Junio C Hamano
2023-07-15 18:24       ` Kousik Sanagavarapu
2023-07-15 18:56         ` Junio C Hamano
2023-07-14 19:20   ` [PATCH v2 3/3] t6300: run describe atom tests on a different repo Kousik Sanagavarapu
2023-07-19 16:15   ` [PATCH v3 0/2] Add new "describe" atom Kousik Sanagavarapu
2023-07-19 16:15     ` [PATCH v3 1/2] ref-filter: add multiple-option parsing functions Kousik Sanagavarapu
2023-07-19 23:23       ` Junio C Hamano
2023-07-20  5:21         ` Junio C Hamano
2023-07-20 16:52         ` Kousik Sanagavarapu
2023-07-20 17:59           ` Junio C Hamano [this message]
2023-07-20 17:42         ` Glen Choo
2023-07-20 20:30           ` Junio C Hamano
2023-07-21 18:26             ` Glen Choo
2023-07-19 16:15     ` [PATCH v3 2/2] ref-filter: add new "describe" atom Kousik Sanagavarapu
2023-07-19 22:56       ` Junio C Hamano
2023-07-20 22:52     ` [PATCH v3 0/2] Add " Junio C Hamano
2023-07-20 23:10       ` Junio C Hamano
2023-07-21  4:17         ` Kousik Sanagavarapu
2023-07-23 16:19     ` [PATCH v4 " Kousik Sanagavarapu
2023-07-23 16:19       ` [PATCH v4 1/2] ref-filter: add multiple-option parsing functions Kousik Sanagavarapu
2023-07-24 17:29         ` Junio C Hamano
2023-07-24 18:12           ` Kousik Sanagavarapu
2023-07-24 20:39             ` Junio C Hamano
2023-07-25 19:27               ` Junio C Hamano
2023-07-23 16:19       ` [PATCH v4 2/2] ref-filter: add new "describe" atom Kousik Sanagavarapu
2023-07-24 17:21         ` Junio C Hamano
2023-07-25 20:51       ` [PATCH v5 0/2] Add " Kousik Sanagavarapu
2023-07-25 20:51         ` [PATCH v5 1/2] ref-filter: add multiple-option parsing functions Kousik Sanagavarapu
2023-07-25 20:51         ` [PATCH v5 2/2] ref-filter: add new "describe" atom Kousik Sanagavarapu
2023-07-25 21:46         ` [PATCH v5 0/2] Add " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq351i4g6s.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chooglen@google.com \
    --cc=christian.couder@gmail.com \
    --cc=five231003@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=hariom18599@gmail.com \
    --cc=siddhartth@google.com \
    --cc=steadmon@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).