Junio C Hamano venit, vidit, dixit 14.08.2009 21:52:

Michael J Gruber [off-list ref] writes:

quoted

Currently, git config dies as soon as there is a parsing error. This is
especially unfortunate in case a user tries to correct config mistakes
using git config -e.

Instead, issue a warning only and treat the rest of the line as a
comment (ignore it). This benefits not only git config -e users.

... a broken sentence in the middle?  I would have expected the "not only"
followed by "but also"; the question is "but also what?"

I don't see any broken sentence here. "Benefit" is a verb as well as a noun.

"not only git config -e users" but also everyone else: Unparseable lines
are ignored, the rest is parsed.

Hopefully the benefit is not that it now allows all the other commands to
cause unspecified types of damage to the repository by following iffy
settings obtained from a broken configuration file.

The only possible problem that I see is when a section heading is not
parsed because of a forgotten "[", say, and the following settings are
put in the wrong section because of that.

quoted

Reported-by: David Reitter <redacted>
Signed-off-by: Michael J Gruber <redacted>

quoted

Test had to be adjusted as well.

The change to the test demonstrates the issue rather well.  The check()
shell function does not check the exit value from "git config --get", but
in a real script that cares to check and stop on error, this change will
now let the script go on, leaving the breakage unnoticed.  I suspect
command implemented in C, that call git_config(), will also have the same
issue, and I cannot convince myself this is a good change in general,
outside the scope of helping "git config -e".

But I may be being overly cautious.

My first version had the lenient mode for "git config -e" only, which
required a new global int (or, alternatively, changing all callers).

One could issue a warning and return -1 rather than success. I'm afraid
git_config() callers don't check return values but rely on git_config()
dieing in case there are parsing problems.

By the way, why did you have to change s/echo/printf/?  Can't you give two
lines in a single argument without "\n" escape?

Because "printf" is more portable then "echo -e". At least I hope so ;)
[ One could use a "here document", of course. Is that preferable? ]

Michael