Re: [PATCH v3] var: support broken-down idents, default key, multiple args, and -z

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

>>   	if (value)
>> -		printf("%s=%s\n", var, value);
>> +		printf("%s=%s%c", var, value, eol);
>
> A key can contain "=" so this format is ambiguous. When the user passes 
> "-z" we should use the same format as "git config list -z" which avoids 
> that ambiguity
>
> 	printf("%s%c%s%c", var, eol == '\n' ? '=' : '\n', value, eol);
>
>>   	else
>> -		printf("%s\n", var);
>> +		printf("%s%c", var, eol);
>
> It would be worth checking what "git config list -z" does when there is 
> no value and matching that. Does it print "key\n\0", or "key\0"?

By "key" do you mean "var"?  The namespace of "var" for "git var"
proper (like GIT_COMMITTER_IDENT) are very much under our control,
but it also gives all the configuration variables, whose names can
indeed have '=' in a three-level varlable name.  This is an
excellent suggestion.

> I think we should break out of the loop when arg is "--", or does not 
> begin with '-', and treat the rest as variable names to print. There is 
> not need to support "git var GIT_AUTHOR_NAME -z GIT_AUTHOR_EMAIL" in a 
> plumbing command.

Not limited to plumbing, but anywhere in Git.  Let's stick to and
force users adopt the simple rule that "git help cli" gives them.
Options first and then args, among which revs coe first and then
paths after disambiguating "--".  I know as historical wart some
commands may take dashed options after args, but I am fine if we
tightened the rule at Git 3.0 boundary to more strictly enforced
the option/argument ordering rule.

>> [...]
>> -	printf("%s\n", val);
>> -	free(val);
>> +		printf("%s%c", val, null_term ? '\0' : '\n');
>
> Multi-valued variables are a bit of a problem here, they're built on the 
> assumption that the individual values do not contain a newline, but as 
> they are paths I'm not sure that is necessarily true. With -z it would 
> be better to print '\0' after each value as we do in list_vars(). 
> Ideally we wouldn't use a single string to pass multiple values around, 
> but a simple fix would be to use '\0' to separate the individual values 
> instead of '\n' so that we can split them unambiguously when we print them.

Hmph, what does "git config -l" do for multi-valued keys?  We should
mimick it, I would think.

Another thing that might be worth doing is to see if we can separate
out "git config -l" handling out of "git var" with a breaking change
at big version boundary.

> Thanks for working on this, being able to specify multiple variables 
> that are printed unambiguously is a really useful improvement.

Indeed.  Thanks, both.