On Sat, May 11, 2013 at 11:59:36AM +0200, Jeff King wrote: > On Sat, May 11, 2013 at 10:55:31AM +0200, Heiko Voigt wrote: > > We where not outputting to stdout before so the test is correct there as > > well. I extended the test when implementing the non-dying version of > > git_config_with_options() so I could see the test fail. If just > > returning the error it would still output the values read until then. So > > if you think that it belongs into the initial version of this test > > (maybe including some comment why we need an extra parseable value) I > > am happy to move it there. > > From what you wrote above, it sounds like we would expect git-config to > write out some.value before failing on some.error. But that isn't what > the test is checking, is it? It is checking that nothing is output. > > I do not think the output matters much either way when git-config has > failed; if it returns a non-zero exit code, then the results of stdout > cannot be trusted (after all, it may have been killed by signal after it > had written out half of the output). > > Still slightly puzzled... Well before my "do not die patch" there was no output to stdout since git already died during reading. Afterwards there would be output of values to stdout until the error occurred and then the error code would be returned. So my intend was to keep the output the same. Since the blob reading is new I thinks its fine either way. So currently I am thinking about just removing the whole "stop to write anything on error" part of this patch and just return the error. What do you think? Cheers Heiko -- 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