Re: [RFC] Git config file reader in Perl (WIP)

From: Eric Wong <normalperson@yhbt.net>
Date: Jan 16, 2007, 19:53 UTC
Message-ID: <20070116195303.GB1444@localdomain>
In-Reply-To: <Pine.LNX.4.63.0701161129310.22628@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> I'd like that code to be simpler, though. Way simpler.

I've tried, but I'm not sure how much farther I can go.

Show 9 quoted lines

> > +	{ "perl",
> > +		"\%git_config = (\n", ");\n",
> 0> +		"\t",
> > +		" => [\n", "\t],\n",
> > +		"\t\t", ",\n",
> > +		"'true'",
> > +		perl_quote_print, perl_quote_print },
> 
> The two quote members seem to be the same for _all_ three languages.

Yes, I was trying to imagine a corner case where quoting for keys could be different than values.

I know Ruby can use :symbols but those don't support '.' and '-' as far as I know, Perl doesn't require quoting for keys if they match ^-?\w+. I guess just using quoted strings as keys is fine enough. The patch below uses the same quote operator for both keys and values.

Show 6 quoted lines

> > +	{ "python",
> > +		"git_config = {\n", "}\n",
> > +		"    ",
> 
> I don't understand why you do not consolidate that into using tabs for 
> _all_ backends?

I wanted to make things look familiar to people using those languages.

Most Ruby programmers I've seen use 2-space indents. I myself have given into using them when I write Ruby.

Python doesn't seem to care about indentation for data structures, but I think Python programmers prefer spaces for indentation. I'm not very experienced with Python, however.

Tabs are my own personal preference for Perl; but indentation is very inconsistent in Perl code I've looked at :/. perlstyle(1) actually recommends 4 space indents...

On the other hand, we are writing for interpreters and not humans. So maybe just using tabs is good enough (some formatting makes debugging easier, so I'm not putting everything on one line :).

Show 18 quoted lines

> > +static int show_lang_config(const char *key_, const char *value_)
> > +{
> > +	if (last_key) {
> > +		if (strcmp(last_key, key_)) {
> > +			free(last_key);
> > +			fputs(lang->array_end, stdout);
> > +			goto new_key;
> > +		}
> > +	} else {
> > +new_key:
> > +		last_key = xstrdup(key_);
> > +		fputs(lang->key_prefix, stdout);
> > +		lang->quote_key_fn(stdout, key_);
> > +		fputs(lang->array_start, stdout);
> > +	}
> 
> So this makes _all_ config vars arrays? It is consistent, yes... but it is 
> also ugly, no?

Somewhat ugly, yes, but I think returning everything as an array would make things easier for code using the data structures. They could always just reference the first element if they didn't want the array instead of having to find the type with ref() or .kind_of?

Show 16 quoted lines

> > +static int show_lang_config_all(const char *lang_name)
> > +{
> > +	int i, rv;
> > +	for (i = ARRAY_SIZE(lang_dump_defs); --i >= 0; ) {
> > +		if (strcmp(lang_name, lang_dump_defs[i].name))
> > +			continue;
> > +		lang = lang_dump_defs + i;
> 
> IMHO this would be much easier to read using a path_list:
> 
> 	struct path_list_item *item = path_list_lookup(lang_name, &langs);
> 
> 	if (item == NULL)
> 		return -1;
> 
> 	lang = item->util;

Ah, I didn't know about path_list_lookup(). Now that I know about it, I don't think it's worth it to create the extra data structures around it. We can just as easily switch to bsearch(3) when we add more languages.

Show 9 quoted lines

> > +		fputs(lang->decl_start, stdout);
> > +		rv = git_config(show_lang_config);
> > +		if (last_key) {
> > +			free(last_key);
> > +			last_key = NULL;
> > +			fputs(lang->array_end, stdout);
> > +			fputs(lang->decl_end, stdout);
> 
> If the config is empty, no decl_end is printed, right?

Good catch.  Thanks.

Show 6 quoted lines

> > +		}
> > +		return rv;
> > +	}
> > +	fputs("Dumping config to '%s' is not yet supported", stderr);
> > +	return -1;
> > +}

--- a/builtin-repo-config.c
+++ b/builtin-repo-config.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "quote.h"
 
 static const char git_config_set_usage[] =
 "git-repo-config [ --global ] [ --bool | --int ] [--get | --get-all | --get-regexp | --replace-all | --add | --unset | --unset-all] name [value [value_regex]] | --rename-section old_name new_name | --list";
@@ -14,6 +15,89 @@ static int do_not_match;
 static int seen;
 static enum { T_RAW, T_INT, T_BOOL } type = T_RAW;
 
+struct lang_dump {
+	const char *name;
+	const char *decl_start;
+	const char *decl_end;
+	const char *key_prefix;
+	const char *array_start;
+	const char *array_end;
+	const char *val_prefix;
+	const char *val_suffix;
+	const char *true_val; /* should already be quoted, if needed */
+	void (*quote_fn)(FILE *, const char*);
+};
+static char *last_key;
+static struct lang_dump *lang;
+static struct lang_dump lang_dump_defs[] = {
+	{ "perl",
+		"\%git_config = (\n", ");\n",
+		"\t",
+		" => [\n", "\t],\n",
+		"\t\t", ",\n",
+		"'true'",
+		perl_quote_print },
+	{ "python",
+		"git_config = {\n", "}\n",
+		"    ",
+		" : [\n", "    ],\n",
+		"        ", ",\n",
+		"True",
+		python_quote_print },
+	{ "ruby", /* Ruby is very Perl-like */
+		"git_config = {\n", "}\n",
+		"  ",
+		" => [\n", "  ],\n",
+		"    ", ",\n",
+		"true",
+		perl_quote_print },
+};
+
+static int show_lang_config(const char *key_, const char *value_)
+{
+	if (last_key) {
+		if (strcmp(last_key, key_)) {
+			free(last_key);
+			fputs(lang->array_end, stdout);
+			goto new_key;
+		}
+	} else {
+new_key:
+		last_key = xstrdup(key_);
+		fputs(lang->key_prefix, stdout);
+		lang->quote_fn(stdout, key_);
+		fputs(lang->array_start, stdout);
+	}
+	fputs(lang->val_prefix, stdout);
+	if (value_)
+		lang->quote_fn(stdout, value_);
+	else
+		fputs(lang->true_val, stdout);
+	fputs(lang->val_suffix, stdout);
+	return 0;
+}
+
+static int show_lang_config_all(const char *lang_name)
+{
+	int i, rv;
+	for (i = ARRAY_SIZE(lang_dump_defs); --i >= 0; ) {
+		if (strcmp(lang_name, lang_dump_defs[i].name))
+			continue;
+		lang = lang_dump_defs + i;
+		fputs(lang->decl_start, stdout);
+		rv = git_config(show_lang_config);
+		if (last_key) {
+			free(last_key);
+			last_key = NULL;
+			fputs(lang->array_end, stdout);
+		}
+		fputs(lang->decl_end, stdout);
+		return rv;
+	}
+	fputs("Dumping config to '%s' is not yet supported", stderr);
+	return -1;
+}
+
 static int show_all_config(const char *key_, const char *value_)
 {
 	if (value_)
@@ -138,6 +222,8 @@ int cmd_repo_config(int argc, const char **argv, const char *prefix)
 			type = T_BOOL;
 		else if (!strcmp(argv[1], "--list") || !strcmp(argv[1], "-l"))
 			return git_config(show_all_config);
+		else if (!strncmp(argv[1], "--dump=", 7))
+			return show_lang_config_all(argv[1] + 7);
 		else if (!strcmp(argv[1], "--global")) {
 			char *home = getenv("HOME");
 			if (home) {

-- 
Eric Wong