Re: git silently ignores include directive with single quotes

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sep 10, 2018, 18:35 UTC
Message-ID: <20180910183557.GD26356@aiede.svl.corp.google.com>
In-Reply-To: <xmqqa7optdbs.fsf@gitster-ct.c.googlers.com>

Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@gmail.com> writes:

Show 13 quoted lines

>>  1. Treat single-quote as worth quoting in config.c::write_pair (line
>>     2516).  This would already help with the original issue, since the
>>     config would say
>>
>> 	[foo]
>> 		bar = \'baz\'
>>
>>     allowing a quick diagnosis.
>
> I am mildly against this, as long as you feel that all the remaining
> steps need to be marked with "(optional)", because this will give
> readers an impression that somehow single-quote is special.  If we
> do not intend to make it special at all, we shouldn't.

That's fair, especially because it would be inconsistent with shell command language, where single-quote inside double quotes is not special:

	$ printf '%s\n' "\'"
	\'

(I realize that backslash means something different in Git config; I'm just saying it would be another source of cognitive dissonance.)

Updated proposal:

  1. Treat strings starting or ending with single-quote as worth
     quoting in config.c::write_pair (line 3269).  This would already
     help with the original issue, since the config would say

	[foo]
		bar = "'baz'"

     allowing a quick diagnosis.

  2. (optional) Warn if a value is surrounded in single-quotes,
     encouraging using surrounding double-quotes to disambiguate.

  3. (optional) Error out if a value is surrounded in single-quotes,
     encouraging replacing with or surrounding with double-quote,
     depending on the user's intention.

  4. (optional) Start treating wrapping single-quotes specially
     somehow.

As before, I think step 1 is a good idea, but I'm not convinced about any of the later steps.

Thanks, Jonathan