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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 17, 2005, 21:10 UTC
Message-ID: <Pine.LNX.4.63.0511172200490.18177@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <7v1x1ful7j.fsf@assigned-by-dhcp.cox.net>

Hi,

[taking the discussion back to public]

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

Show 8 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 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.

Show 10 quoted lines

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

Show 6 quoted lines

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

Show 6 quoted lines

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