Re: [PATCH] Fix git_config_set() for mean cases

Hi,

[taking the discussion back to public]

On Thu, 17 Nov 2005, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

Also, I felt that a function like git_config_set() was lacking for too 
long from git. Maybe git-init-db does not need that flag, but I guess that 
we'd want to write config variables comfortably from C at some stage.

Yes.  But I hesitate to throw in an interface that dismisses the
multivalued key problem without thinking things through first.

Okay, I gave it some thought. See below for my points.

BTW, I think this discussion should be thrown back to the
public.  It went private by accident.

Hereby done.

Mind rehashing the discussion so far in the open?

The main points I can think of are:

 - configuration file format and semantics cleanups (methinks
   it is premature to talk about implementation and interface
   before discussing this).

   - allowing sectionless variables seems to be a bug.  disallow
     it.

 Agree.

   - section names are alnum (i.e. they can start with a digit)
     while variable names are alpha+alnum.  is this what we
     want?  otherwise how should the rule be changed?

It seems natural to make variables start with alpha, since people are so 
used to it. It would not hurt the parsing to make them possibly start with 
a digit, but I think it's okay to let it be as it is.

   - multivalued variables.  it is nice to have such, and is
     easier to read by humans (methinks).  it is not how other
     systems do .ini format, and the tools can live with single
     value and parse it into list of values (methinks that is
     what you are saying).  which way are we going?  does the
     order matter in such list valued variable in either case?

To recap:

        [proxy]
                command="ssh" for "ssh://kernel.org/"
                command="proxy-command" for kernel.org
                command="myprotocol-command" for "my://"

It should be possible to set only the command for kernel.org while leaving 
the others. I actually implemented a function

	git_config_set_multivar(const char* name, const char* value,
		const char* value_regex);

which you can call like this:

	git_config_set_multivar(
		"proxy.command",
		"\"rsh\" for kernel.org",
		"for kernel.org$");

to change just the second command. You can use that function also to 
unset (i.e. remove) an entry:

	git_config_set_multivar(
		"proxy.command",
		NULL,
		"for \"ssh://kernel.org\"$");

 - having a way for scripts to query and update the config file
   programatically is a good thing, both from C level and script
   level (i do not think there is a disagreement between us on
   this point).

There is no disagreement.

   - _if_ we do multivalued variables, what is the interface to
     append, replace, and remove one of the values?  (we may be
     able to get away by only supporting "replace the whole
     thing", in which case the program can do git_config_get()
     to grab the list, manipulate the list and feed the updated
     list to git_config_set()).

See above. For clarities sake, I left "git_config_set(name,value)" as 
shortcut to "git_config_set_multivar(name,value,NULL)", i.e. calling the 
multivar version with value_regex==NULL actually reverts to singlevar 
version.

I'll send out the patches in a few minutes (have to get them into shape 
first).

Ciao,
Dscho