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

Re: [PATCH v2 1/7] git-sh-setup: remove unused git_pager() function

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Sep 7, 2021, 09:41 UTC
Message-ID
<8912317f-7eb0-edae-29e3-2e05099bc696@gmail.com>
In-Reply-To
<87czplnxn3.fsf@evledraar.gmail.com>
On 06/09/2021 23:27, Ævar Arnfjörð Bjarmason wrote:
Show 35 quoted lines
> 
> On Mon, Sep 06 2021, Phillip Wood wrote:
> 
>> Hi Ævar
>>
>> On 06/09/2021 08:05, Ævar Arnfjörð Bjarmason wrote:
>>> Remove the git_pager() function last referenced by non-test code in
>>> 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
>>> We can also remove the test for this added in 995bc22d7f8 (pager:
>>> move
>>> pager-specific setup into the build, 2016-08-04), the test that
>>> actually matters is the one added in e54c1f2d253 (pager: set LV=-c
>>> alongside LESS=FRSX, 2014-01-06) just above the removed test.
>>> I.e. we don't care if the "LESS" and "LV" variables are set by
>>> git-sh-setup anymore, no built-in uses them, we do care that pager.c
>>> sets them, which we still test for.
>>
>> git_pager() might not be documented but I think it is useful for
>> script authors and I wouldn't be surprised if someone out there is
>> using it. The same goes for peel_committish(). It does not seem like a
>> huge maintenance burden to keep and maybe document these two
>> functions.
> 
> The git_pager() and peel_committish() seem to thoroughly be in the same
> camp as the now-removed git-parse-remote.sh (see a89a2fbfccd
> (parse-remote: remove this now-unused library, 2020-11-14)) and say its
> get_remote_merge_branch(). I.e. we carried it for a while, but the
> function was never publicly documented.
> 
> I think rather than document these it makes sense to just kick that
> maintenance burden over to whoever decided they'd rely on undocumented
> shellscript functions git was shipping.
> 
> In these cases they can rather easily use the documented GIT_PAGER
> environment variable directly, 

No, they need to know to call 'git var GIT_PAGER' rather than using the environment variable directly to pick up core.pager and they should be checking whether stdout is a tty. That is why this function existed and we didn't just check the value of GIT_PAGER in our scripts

> and their own invocation of "git rev-parse" for peel_committish().

The reason the function exists is that you cannot just call 'git rev-parse $OID^{commit}' if $OID starts with :/

I'm not sure what the maintenance burden of keeping these functions is that makes it worth removing them

Best Wishes
Phillip
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 26 of 39 in “remove dead shell code”
  1. 0/9 remove dead shell codeÆvar Arnfjörð Bjarmason, Sep 2, 2021
  2. 1/9 git-sh-setup: remove unused set_reflog_action() functionÆvar Arnfjörð Bjarmason, Sep 2, 2021
  3. 2/9 git-sh-setup: remove unused git_editor() functionÆvar Arnfjörð Bjarmason, Sep 2, 2021
  4. 3/9 git-sh-setup: remove unused git_pager() functionÆvar Arnfjörð Bjarmason, Sep 2, 2021
  5. Philippe BlainSep 2, 2021
  6. Andrei RybakSep 2, 2021
  7. 4/9 git-sh-setup: remove unused sane_egrep() functionÆvar Arnfjörð Bjarmason, Sep 2, 2021
  8. 5/9 git-sh-setup: remove unused require_work_tree_exists() functionÆvar Arnfjörð Bjarmason, Sep 2, 2021
  9. 6/9 git-sh-setup: move create_virtual_base() to mergetools/p4mergeÆvar Arnfjörð Bjarmason, Sep 2, 2021
  10. 7/9 git-sh-setup: move peel_committish() function to git-subtree.shÆvar Arnfjörð Bjarmason, Sep 2, 2021
  11. 8/9 git-bisect: remove unused SHA-1 $x40 shell variableÆvar Arnfjörð Bjarmason, Sep 2, 2021
  12. 9/9 test-lib: remove unused $_x40 and $_z40 variablesÆvar Arnfjörð Bjarmason, Sep 2, 2021
  13. Peter BaumannSep 2, 2021
  14. Junio C HamanoSep 2, 2021
  15. Junio C HamanoSep 2, 2021
  16. Carlo ArenasSep 2, 2021
  17. Junio C HamanoSep 2, 2021
  18. Ævar Arnfjörð BjarmasonSep 2, 2021
  19. Junio C HamanoSep 2, 2021
  20. 0/7 remove dead & undocumented shell codeÆvar Arnfjörð Bjarmason, Sep 6, 2021
  21. 2/7 git-sh-setup: remove unused sane_egrep() functionÆvar Arnfjörð Bjarmason, Sep 6, 2021
  22. 3/7 git-sh-setup: move peel_committish() function to git-subtree.shÆvar Arnfjörð Bjarmason, Sep 6, 2021
  23. 1/7 git-sh-setup: remove unused git_pager() functionÆvar Arnfjörð Bjarmason, Sep 6, 2021
  24. Phillip WoodSep 6, 2021
  25. Ævar Arnfjörð BjarmasonSep 6, 2021
  26. Phillip WoodSep 7, 2021
  27. Ævar Arnfjörð BjarmasonSep 7, 2021
  28. Junio C HamanoSep 7, 2021
  29. Ævar Arnfjörð BjarmasonSep 7, 2021
  30. 5/7 git-sh-setup: remove unused "pull with rebase" messageÆvar Arnfjörð Bjarmason, Sep 6, 2021
  31. 4/7 git-sh-setup: clear_local_git_env() function to git-submodule.shÆvar Arnfjörð Bjarmason, Sep 6, 2021
  32. 6/7 git-bisect: remove unused SHA-1 $x40 shell variableÆvar Arnfjörð Bjarmason, Sep 6, 2021
  33. 7/7 test-lib: remove unused $_x40 and $_z40 variablesÆvar Arnfjörð Bjarmason, Sep 6, 2021
  34. 0/4 remove dead & internal-only shell codeÆvar Arnfjörð Bjarmason, Sep 11, 2021
  35. 2/4 git-sh-setup: remove unused "pull with rebase" messageÆvar Arnfjörð Bjarmason, Sep 11, 2021
  36. 1/4 git-submodule: remove unused is_zero_oid() functionÆvar Arnfjörð Bjarmason, Sep 11, 2021
  37. Junio C HamanoSep 13, 2021
  38. 3/4 git-bisect: remove unused SHA-1 $x40 shell variableÆvar Arnfjörð Bjarmason, Sep 11, 2021
  39. 4/4 test-lib: remove unused $_x40 and $_z40 variablesÆvar Arnfjörð Bjarmason, Sep 11, 2021

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.