Jeff King <peff@xxxxxxxx> writes: > On Wed, Feb 11, 2015 at 01:15:05AM +0530, Tanay Abhra wrote: > >> I just saw your mail late in the night (I didn't had net for a week). >> This patch just squelches the error message, I will take a better >> look tomorrow morning. > > Thanks, this is probably a good first step. We can worry about making > the config look actually _work_ as the next step (which does not even > have to happen right now; it is not like it hasn't been this way since > the very beginning of git). I agree this is probably a good first step in the right direction. As to the implementation, there are a few minor things I would change, but they are both minor: - "defective" may want to be a bit more descriptive to clarify what kind fo defect is undesired. In the context of this patch, I think Tanay meant (syntactically) "malformed", perhaps? - "int show_err" should be "unsigned flags" with its bit 01 defined to be used as QUIET bit. > Another option for this first step would be to actually make > git_config_parse_key permissive, rather than just squelching the > error. That is, to actually look up pager.under_score rather than > silently erroring out with an invalid key whenever we are reading > (whereas on the writing side, we _do_ want to make sure we enforce > syntactic validity). I doubt it matters, much, though. Sensible. > I was tempted to also add something like: > > test_expect_failure TTY 'command with underscores can override pager' ' > test_config pager.under_score "sed s/^/paged://" && > git --exec-path=. under_score >actual && > echo paged:ok >expect && > test_cmp expect actual > ' > > but I am not sure it is worth adding the test, even as a placeholder. > Unless we are planning to relax the config syntax, the correct spelling > is more like "pager.under_score.command". It's probably better to just > add the test along with the code when we know what the final form will > look like. Concurred. -- 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