On Fri, Nov 19, 2010 at 11:16:53AM -0600, Jeffrey Middleton wrote: > Okay, that makes sense, I think. But execv_dashed_external does > currently commit the pager choice, just without looking it up from the > config. This means that, for example, git --paginate > <nonexistent-command> does invoke the pager. Is that intended? I don't > think I see any tests covering the combination of pager.<cmd> for > anything but builtins. Hmm. You're right, I should have looked at the current code instead of digging in the history. :) Looks like it was part of some pager work Jonathan did this summer. I think there may have been issues in the past, too, with actually looking at config before exec'ing the external, but those seem cleared up, too. So in theory you just need this: diff --git a/git.c b/git.c index 0409ac9..bb2c726 100644 --- a/git.c +++ b/git.c @@ -438,6 +438,8 @@ static void execv_dashed_external(const char **argv) const char *tmp; int status; + if (use_pager == -1) + use_pager = check_pager_config(argv[0]); commit_pager_choice(); strbuf_addf(&cmd, "git-%s", argv[0]); But beyond seeing that it does in fact turn on the pager for an external command, I've done no testing. As far as "git -p bogus", I guess we have accepted that it will start a pager. These days we at least send stderr to the pager, too, so the error message won't go unseen. > Also, instead of having git implement "look up in PATH" internally, > would it make sense to simply commit the pager choice, try the execv, > then uncommit the pager choice if the command was not found? No. The reason we push commit_pager_choice off to the last minute is that you can't uncommit it gracefully. It actually execs the pager, which may then do things to the terminal outside of our control. -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