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). 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. Such a lookup would never succeed, because the config file parser will also not allow it. So assuming the syntactic rules here match what the config file parser does, they are at worst redundant. > builtin/config.c | 2 +- > cache.h | 2 +- > config.c | 19 ++++++++++++------- > 3 files changed, 14 insertions(+), 9 deletions(-) Here's a test that can be squashed in: diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index da958a8..a28a2fd 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -447,4 +447,14 @@ test_expect_success TTY 'external command pagers override sub-commands' ' test_cmp expect actual ' +test_expect_success 'command with underscores does not complain' ' + write_script git-under_score <<-\EOF && + echo ok + EOF + git --exec-path=. under_score >actual 2>&1 && + echo ok >expect && + test_cmp expect actual +' + + test_done 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. -Peff -- 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