Re: [PATCH] Add test case for git-config-set

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

I am not so sure about this case.  Shouldn't this remove both?

For example, if a Porcelain wants to force pull.twohead to be resolve and nothing else, and it wants to do it unconditionally, it would first want to empty whatever multivalue there are currently, and then insert its own, and I'd imagine the way to say that would be like this:

	git-config-set --unset pull.twohead '^'
        git-config-set pull.twohead resolve

More simply (I do not think you have a test case for this):

        git-config-set pull.twohead resolve '^'

I think it is the easiest to explain and understand the semantics of config_set_multivalue if it were to first remove all existing key-value for matching ones, and then insert what was provided by the user.

Extending that multivalue example a bit more, I think it is a bit cumbersome for a Porcelain to set pull.twohead to recursive and then resolve, with your interface. Even if you had the emptying behaviour I suggested above, you would have to say something awkard like this:

	git-config-set --unset pull.twohead '^'
        git-config-set pull.twohead recursive
        git-config-set pull.twohead resolve no-such-value-should-be-there

Maybe we could have the shell-level interface like this:

	git-config-set [--remove rx] section.key [value...]

When --remove rx is specified, the command first removes existing multivalue for the given section.key that match rx, and then insert given value(s); not giving any values amounts to --unset. Not giving --remove rx is the same as giving a regexp that matches all multivalues. So the simplest:

	git-config-set section.key value

becomes a single-value assignment (insert-or-replace),

	git-config-set section.key

confusingly enough is --unset (we probably would want to require an explicit command line noise-word "--unset" in this case). And "replacing with two values regardless of whatever there are currently" naturally becomes:

	git-config-set pull.twohead recursive resolve

The C-level interface would become something like:

	git_config_set_multivar(const char *key,
        			const char *remove_value_regex,
                                const char **values)

where values is a NULL terminated list of values.

BTW, do we want to remove the section after removing the last key and making it empty?