Skip to content

Commit 73e25e7

Browse files
pcloudsgitster
authored andcommitted
git --paginate: do not commit pager choice too early
When git is passed the --paginate option, starting up a pager requires deciding what pager to start, which requires access to the core.pager configuration. At the relevant moment, the repository has not been searched for yet. Attempting to access the configuration at this point results in git_dir being set to .git [*], which is almost certainly not what was wanted. In particular, when run from a subdirectory of the toplevel, git --paginate does not respect the core.pager setting from the current repository. [*] unless GIT_DIR or GIT_CONFIG is set So delay the pager startup when possible: 1. run_argv() already commits pager choice inside run_builtin() if a command is found. For commands that use RUN_SETUP, waiting until then fixes the problem described above: once git knows where to look, it happily respects the core.pager setting. 2. list_common_cmds_help() prints out 29 lines and exits. This can benefit from pagination, so we need to commit the pager choice before writing this output. Luckily ‘git’ without subcommand has no other reason to access a repository, so it would be intuitive to ignore repository-local configuration in this case. Simpler for now to choose a pager using the funny code that notices a repository that happens to be at .git. That this accesses a repository when it is very convenient to is a bug but not an important one. 3. help_unknown_cmd() prints out a few lines to stderr. It is not important to paginate this, so don’t. Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]> Signed-off-by: Jonathan Nieder <[email protected]> Acked-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent bce2c9a commit 73e25e7

File tree

2 files changed

+47
-13
lines changed

2 files changed

+47
-13
lines changed

git.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,12 +511,12 @@ int main(int argc, const char **argv)
511511
argv++;
512512
argc--;
513513
handle_options(&argv, &argc, NULL);
514-
commit_pager_choice();
515514
if (argc > 0) {
516515
if (!prefixcmp(argv[0], "--"))
517516
argv[0] += 2;
518517
} else {
519518
/* The user didn't specify a command; give them help */
519+
commit_pager_choice();
520520
printf("usage: %s\n\n", git_usage_string);
521521
list_common_cmds_help();
522522
printf("\n%s\n", git_more_info_string);

t/t7006-pager.sh

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,21 @@ test_PAGER_overrides() {
244244
}
245245

246246
test_core_pager_overrides() {
247+
if_local_config=
248+
used_if_wanted='overrides PAGER'
249+
test_core_pager "$@"
250+
}
251+
252+
test_local_config_ignored() {
253+
if_local_config='! '
254+
used_if_wanted='is not used'
255+
test_core_pager "$@"
256+
}
257+
258+
test_core_pager() {
247259
parse_args "$@"
248260

249-
$test_expectation TTY "$cmd - core.pager overrides PAGER" "
261+
$test_expectation TTY "$cmd - repository-local core.pager setting $used_if_wanted" "
250262
unset GIT_PAGER;
251263
rm -f core.pager_used ||
252264
cleanup_fail &&
@@ -255,14 +267,26 @@ test_core_pager_overrides() {
255267
export PAGER &&
256268
git config core.pager 'wc >core.pager_used' &&
257269
$full_command &&
258-
test -e core.pager_used
270+
${if_local_config}test -e core.pager_used
259271
"
260272
}
261273

262274
test_core_pager_subdir() {
275+
if_local_config=
276+
used_if_wanted='overrides PAGER'
277+
test_pager_subdir_helper "$@"
278+
}
279+
280+
test_no_local_config_subdir() {
281+
if_local_config='! '
282+
used_if_wanted='is not used'
283+
test_pager_subdir_helper "$@"
284+
}
285+
286+
test_pager_subdir_helper() {
263287
parse_args "$@"
264288

265-
$test_expectation TTY "$cmd - core.pager from subdirectory" "
289+
$test_expectation TTY "$cmd - core.pager $used_if_wanted from subdirectory" "
266290
unset GIT_PAGER;
267291
rm -f core.pager_used &&
268292
rm -fr sub ||
@@ -277,7 +301,7 @@ test_core_pager_subdir() {
277301
cd sub &&
278302
$full_command
279303
) &&
280-
test -e core.pager_used
304+
${if_local_config}test -e core.pager_used
281305
"
282306
}
283307

@@ -296,6 +320,20 @@ test_GIT_PAGER_overrides() {
296320
"
297321
}
298322

323+
test_doesnt_paginate() {
324+
parse_args "$@"
325+
326+
$test_expectation TTY "no pager for '$cmd'" "
327+
rm -f GIT_PAGER_used ||
328+
cleanup_fail &&
329+
330+
GIT_PAGER='wc >GIT_PAGER_used' &&
331+
export GIT_PAGER &&
332+
$full_command &&
333+
! test -e GIT_PAGER_used
334+
"
335+
}
336+
299337
test_default_pager expect_success 'git log'
300338
test_PAGER_overrides expect_success 'git log'
301339
test_core_pager_overrides expect_success 'git log'
@@ -305,19 +343,15 @@ test_GIT_PAGER_overrides expect_success 'git log'
305343
test_default_pager expect_success 'git -p log'
306344
test_PAGER_overrides expect_success 'git -p log'
307345
test_core_pager_overrides expect_success 'git -p log'
308-
test_core_pager_subdir expect_failure 'git -p log'
346+
test_core_pager_subdir expect_success 'git -p log'
309347
test_GIT_PAGER_overrides expect_success 'git -p log'
310348

311349
test_default_pager expect_success test_must_fail 'git -p'
312350
test_PAGER_overrides expect_success test_must_fail 'git -p'
313-
test_core_pager_overrides expect_success test_must_fail 'git -p'
314-
test_core_pager_subdir expect_failure test_must_fail 'git -p'
351+
test_local_config_ignored expect_failure test_must_fail 'git -p'
352+
test_no_local_config_subdir expect_success test_must_fail 'git -p'
315353
test_GIT_PAGER_overrides expect_success test_must_fail 'git -p'
316354

317-
test_default_pager expect_success test_must_fail 'git -p nonsense'
318-
test_PAGER_overrides expect_success test_must_fail 'git -p nonsense'
319-
test_core_pager_overrides expect_success test_must_fail 'git -p nonsense'
320-
test_core_pager_subdir expect_failure test_must_fail 'git -p nonsense'
321-
test_GIT_PAGER_overrides expect_success test_must_fail 'git -p nonsense'
355+
test_doesnt_paginate expect_success test_must_fail 'git -p nonsense'
322356

323357
test_done

0 commit comments

Comments
 (0)