git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 00/10] tag: only respect `pager.tag` in list-mode

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 20, 2017, 22:27 UTC
Message-ID
<xmqq379qkalj.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<cover.1500321657.git.martin.agren@gmail.com>
Martin Ågren <martin.agren@gmail.com> writes:
Show 19 quoted lines
> This is the second version of "[PATCH 0/7] tag: more fine-grained
> pager-configuration" [1]. That series introduced `pager.tag.list` to
> address the fact that `pager.tag` can be useful with `git tag -l` but
> actively hostile with `git tag -a`. Thanks to Junio, Peff and Brandon
> for helpful feedback.
>
> After that feedback, v2 drops `pager.tag.list` and instead teaches
> `git tag` to only consider `pager.tag` in list-mode, as suggested by
> Peff.
>
> Patches 1-3/10 replace patch 1/7. They move Documentation/technical/
> api-builtin.txt into builtin.h, tweak the formatting and bring it up to
> date. I may have gone overboard making this 3 patches...
>
> Patches 4-7/10 correspond to patches 2-5/7. `setup_auto_pager()' is now
> much simpler since we do not need to handle "tag.list" with a clever
> fallback strategy. IGNORE_PAGER_CONFIG is now called DELAY_PAGER_CONFIG.
> I now check with pager_in_use() and I moved the handling of `pager.tag`
> a bit further down.

I tend to agree with you that 1-3/10 may be better off being a single patch (or 3/10 dropped, as Brandon is working on losing it nearby). I would have expected 7-8/10 to be a single patch, as by the time a reader reaches 07/10, because of the groundwork laid by 04-06/10, it is obvious that the general direction is to allow the caller, i.e. cmd_tag(), to make a call to setup_auto_pager() only in some but not all circumstances, and 07/10 being faithful to the original behaviour (only to be updated in 08/10) is somewhat counter intuitive. It is not wrong per-se; it was just unexpected.

Show 10 quoted lines
> Patches 8-9/10 teach `git tag` to only respect `pager.tag` in list-mode
> and flip the default value for that config to "on".
>
> Patch 10/10 is somewhat similar to a hunk in patch 2/7, but is now a
> bug-fix instead of a feature. It teaches `execv_dashed_external()` not
> to check `pager.foo` when launching `git-foo` where foo is a builtin.
> I waffled about where to put this patch. Putting it earlier in the
> series as a preparatory step, I couldn't come up with a way of writing a
> test. So patch 8/10 introduces a `test_expect_failure` which this patch
> then fixes.

I haven't thought about ramifications of 9-10/10 to make a comment yet, but overall the series was a pleasant read.

Thanks.
Previous: Junio C HamanoNext: Martin Ågren
Message 48 of 69 in “tag: more fine-grained pager-configuration”
  1. 0/7 tag: more fine-grained pager-configurationMartin Ågren, Jul 10, 2017
  2. 1/7 api-builtin.txt: document SUPPORT_SUPER_PREFIXMartin Ågren, Jul 10, 2017
  3. Brandon WilliamsJul 10, 2017
  4. Martin ÅgrenJul 11, 2017
  5. Brandon WilliamsJul 11, 2017
  6. 2/7 git.c: let builtins opt for handling `pager.foo` themselvesMartin Ågren, Jul 10, 2017
  7. Jeff KingJul 11, 2017
  8. Martin ÅgrenJul 11, 2017
  9. Jeff KingJul 11, 2017
  10. 3/7 git.c: provide setup_auto_pager()Martin Ågren, Jul 10, 2017
  11. Jeff KingJul 11, 2017
  12. Martin ÅgrenJul 11, 2017
  13. 4/7 t7006: add tests for how git tag paginatesMartin Ågren, Jul 10, 2017
  14. 6/7 tag: make git tag -l consider new config `pager.tag.list`Martin Ågren, Jul 10, 2017
  15. Jeff KingJul 11, 2017
  16. Martin ÅgrenJul 11, 2017
  17. 5/7 tag: handle `pager.tag`-configuration within the builtinMartin Ågren, Jul 10, 2017
  18. Jeff KingJul 11, 2017
  19. 7/7 tag: make git tag -l use pager by defaultMartin Ågren, Jul 10, 2017
  20. Jeff KingJul 11, 2017
  21. Junio C HamanoJul 10, 2017
  22. Jeff KingJul 11, 2017
  23. Junio C HamanoJul 11, 2017
  24. Martin ÅgrenJul 12, 2017
  25. Jeff KingJul 11, 2017
  26. Martin ÅgrenJul 11, 2017
  27. Jeff KingJul 11, 2017
  28. 00/10 tag: only respect `pager.tag` in list-modeMartin Ågren, Jul 17, 2017
  29. 01/10 builtin.h: take over documentation from api-builtin.txtMartin Ågren, Jul 17, 2017
  30. 02/10 builtin.h: format documentation-comment properlyMartin Ågren, Jul 17, 2017
  31. 03/10 builtin.h: document SUPPORT_SUPER_PREFIXMartin Ågren, Jul 17, 2017
  32. 04/10 git.c: let builtins opt for handling `pager.foo` themselvesMartin Ågren, Jul 17, 2017
  33. 05/10 git.c: provide setup_auto_pager()Martin Ågren, Jul 17, 2017
  34. Jeff KingJul 31, 2017
  35. Junio C HamanoJul 31, 2017
  36. 06/10 t7006: add tests for how git tag paginatesMartin Ågren, Jul 17, 2017
  37. Jeff KingJul 31, 2017
  38. Junio C HamanoJul 31, 2017
  39. Martin ÅgrenJul 31, 2017
  40. 07/10 tag: handle `pager.tag`-configuration within the builtinMartin Ågren, Jul 17, 2017
  41. 08/10 tag: respect `pager.tag` in list-mode onlyMartin Ågren, Jul 17, 2017
  42. Jeff KingJul 31, 2017
  43. 09/10 tag: change default of `pager.tag` to "on"Martin Ågren, Jul 17, 2017
  44. 10/10 git.c: ignore pager.* when launching builtin as dashed externalMartin Ågren, Jul 17, 2017
  45. Jeff KingJul 31, 2017
  46. Martin ÅgrenJul 31, 2017
  47. Junio C HamanoJul 18, 2017
  48. Junio C HamanoJul 20, 2017
  49. Martin ÅgrenJul 23, 2017
  50. Jeff KingJul 31, 2017
  51. Martin ÅgrenJul 31, 2017
  52. Jeff KingJul 31, 2017
  53. Junio C HamanoJul 31, 2017
  54. 0/7 tag: only respect `pager.tag` in list-modeMartin Ågren, Aug 2, 2017
  55. 1/7 builtin.h: take over documentation from api-builtin.txtMartin Ågren, Aug 2, 2017
  56. Junio C HamanoAug 3, 2017
  57. Martin ÅgrenAug 4, 2017
  58. Junio C HamanoAug 4, 2017
  59. Martin ÅgrenAug 4, 2017
  60. 2/7 git.c: let builtins opt for handling `pager.foo` themselvesMartin Ågren, Aug 2, 2017
  61. 3/7 git.c: provide setup_auto_pager()Martin Ågren, Aug 2, 2017
  62. 4/7 t7006: add tests for how git tag paginatesMartin Ågren, Aug 2, 2017
  63. 5/7 tag: respect `pager.tag` in list-mode onlyMartin Ågren, Aug 2, 2017
  64. 6/7 tag: change default of `pager.tag` to "on"Martin Ågren, Aug 2, 2017
  65. 7/7 git.c: ignore pager.* when launching builtin as dashed externalMartin Ågren, Aug 2, 2017
  66. Junio C HamanoAug 3, 2017
  67. Jeff KingAug 3, 2017
  68. Martin ÅgrenAug 4, 2017
  69. Jeff KingAug 4, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.