@matheusbsbs / matheus-bagliano / commits / 73e25e7cc8

Fork 0

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 <pclouds@gmail.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Jun 26, 2010 at 14:26 UTC 73e25e7cc80e175b5c94678188333e42107fa16e

2 files changed +47 -13

git.c

+1 -1

@@ -511,12 +511,12 @@ int main(int argc, const char **argv)

511 argv++;

512 argc--;

513 handle_options(&argv, &argc, NULL);

514 - commit_pager_choice();

514 if (argc > 0) {

515 if (!prefixcmp(argv[0], "--"))

516 argv[0] += 2;

517 } else {

518 /* The user didn't specify a command; give them help */

519 + commit_pager_choice();

520 printf("usage: %s\n\n", git_usage_string);

521 list_common_cmds_help();

522 printf("\n%s\n", git_more_info_string);

t/t7006-pager.sh

+46 -12

@@ -244,9 +244,21 @@ test_PAGER_overrides() {

244 }

245

246 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() {

259 parse_args "$@"

260

249 - $test_expectation TTY "$cmd - core.pager overrides PAGER" "

261 + $test_expectation TTY "$cmd - repository-local core.pager setting $used_if_wanted" "

262 unset GIT_PAGER;

263 rm -f core.pager_used ||

264 cleanup_fail &&

@@ -255,14 +267,26 @@ test_core_pager_overrides() {

267 export PAGER &&

268 git config core.pager 'wc >core.pager_used' &&

269 $full_command &&

258 - test -e core.pager_used

270 + ${if_local_config}test -e core.pager_used

271 "

272 }

273

274 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() {

287 parse_args "$@"

288

265 - $test_expectation TTY "$cmd - core.pager from subdirectory" "

289 + $test_expectation TTY "$cmd - core.pager $used_if_wanted from subdirectory" "

290 unset GIT_PAGER;

291 rm -f core.pager_used &&

292 rm -fr sub ||

@@ -277,7 +301,7 @@ test_core_pager_subdir() {

301 cd sub &&

302 $full_command

303 ) &&

280 - test -e core.pager_used

304 + ${if_local_config}test -e core.pager_used

305 "

306 }

307

@@ -296,6 +320,20 @@ test_GIT_PAGER_overrides() {

320 "

321 }

322

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 +

337 test_default_pager expect_success 'git log'

338 test_PAGER_overrides expect_success 'git log'

339 test_core_pager_overrides expect_success 'git log'

@@ -305,19 +343,15 @@ test_GIT_PAGER_overrides expect_success 'git log'

343 test_default_pager expect_success 'git -p log'

344 test_PAGER_overrides expect_success 'git -p log'

345 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'

347 test_GIT_PAGER_overrides expect_success 'git -p log'

348

349 test_default_pager expect_success test_must_fail 'git -p'

350 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'

353 test_GIT_PAGER_overrides expect_success test_must_fail 'git -p'

354

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'

356

357 test_done