Nguyen Thai Ngoc Duy wrote: > 2010/4/13 Jonathan Nieder <jrnieder@xxxxxxxxx>: >> @@ -251,7 +251,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv) [...] >> - if (use_pager == -1 && p->option & RUN_SETUP) >> + if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY)) >> use_pager = check_pager_config(p->cmd); >> if (use_pager == -1 && p->option & USE_PAGER) >> use_pager = 1; > > This still leaves a chance of going wrong: when user explicitly gives > "--paginate", use_pager will be 1, but commands like "git init" does > not have RUN_SETUP*. So when setup_pager is called later on, it will > mess things up. I decided not to check_pager_config() unconditionally to avoid breaking ‘git diff’ (especially ‘git diff --exit-code’). Maybe that is too conservative; not sure. The commit message should be more explicit, something like: Eventually, all git commands should check their configuration at start-up. For now, reading configuration before repository discovery is dangerous because it could cause a pager.<cmd> setting from the current repository to be neglected. But for commands with RUN_SETUP or RUN_SETUP_GENTLY set, it is safe. This will not affect commands like "git init" that cannot have RUN_SETUP* set; they will have to be helped separately later. In particular, commands such as "git diff" that use command-specific options to decide whether to search for a repository and whether to paginate output are outside the scope of this change. Guiding principle: better to leave some bugs for later than risk regressions. > This could be solved completely (indeed I have a patch > under testing), but it would require unset_git_directory(), making > this series a bit longer :( I agree; that’s the right way to do it. run_builtin() ought to be relatively sure about whether a repo is going to be searched for. For commands like diff and grep, I think it should be okay to search for a repo unless --no-index is the first argument. Thanks for your thoughtfulness. Jonathan -- 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