> errors will likewise go to the pager. E.g., try "git log --foo". Hope that I didn't take it the wrong way, but I don't think `git log --foo` starts a pager, where the routine `setup_pager()` is put after argv parsing. (checked by `strace`) > So I dunno. I'm not strictly opposed to making things nicer when we can > do so easily. But the endgame of this is probably getting rid of > USE_PAGER entirely and asking each builtin to decide when to commit to > using the pager (presumably after option parsing). > > And even then, it wouldn't apply to commands implemented as an external > process. And of course we can still die(), etc, after starting the > pager. So it would never be totally consistent. Despite the example, it is overall convincing since currently there is no design to ensure the pager consistency. However, it's something we can do right now to make, at least our own builtins, more consistent. > We usually ask for something approaching a legal name, as this sign-off > is supposed to be certifying the DCO (see the "dco" section in > Documentation/SubmittingPatches). Sorry if my first GitGitGadget experience bothers the mailing list, thanks for the reminder. :) > would be missing a spot that needed a new setup_diff_pager() call, and I > suspect we don't have good test coverage here. This is actually my concern as well when I was naively testing the coverage using GDB, which turned out to be quite tedious. Would you consider it's fine to add a pager consistency test for builtins, probably in another patch with regard to `t7006-pager.sh` OR a new test `t7007`? I'll reword and re-signoff this patch as soon as it looks really fine to you. Philip Yung Best Regards
