Junio C Hamano venit, vidit, dixit 14.08.2009 21:52: > Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes: > >> 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. >> Reported-by: David Reitter <david.reitter@xxxxxxxxx> >> Signed-off-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> > >> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html