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

Re: [PATCH 1/2] pager: move pager-specific setup into the build

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 1, 2016, 18:07 UTC
Message-ID
<xmqq37mol5w6.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20160801172459.wz7wigmytxpfvs22@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 17 quoted lines
> I'm not too worried about spaces here. This is a resurrection of an old
> discussion, and in all that time, I think the only realistic suggestions
> for built-in values have been pretty tame.
>
> If this were used to parse arbitrary user-provided runtime values, I'd
> be more concerned. But I'm not sure why we would need that. Your $EDITOR
> example is arbitrary shell code, and we let the shell handle it (modulo
> some efficiency shortcuts). Likewise, fancy runtime things should go in
> GIT_PAGER, where you can not only set options with spaces, but do fancy
> things like pipes, shell functions, etc.
>
> The use of stringify() here is funny to me; I think there is a cpp
> tokenizing step in the middle that will do things like gobble up
> whitespace (but I'm not sure if it has other possible effects). I think
> our more usual method here would be to C-quote in the Makefile (with the
> equivalent of 's/\\/\\\\/g; s/"/\\"/g'), and then pass it to the
> compiler as a string literal, like -DPAGER_ENV=\"$(PAGER_ENV_CQ_SQ\".
All sensible arguments, including the rationale to reject 2/2.
Thanks.
Previous: Jeff KingNext: Duy Nguyen
Message 8 of 32 in “add PAGER_ENV to build and core.pagerEnv to config”
  1. 0/2 add PAGER_ENV to build and core.pagerEnv to configEric Wong, Aug 1, 2016
  2. 1/2 pager: move pager-specific setup into the buildEric Wong, Aug 1, 2016
  3. brian m. carlsonAug 1, 2016
  4. Eric WongAug 1, 2016
  5. Jakub NarębskiAug 1, 2016
  6. brian m. carlsonAug 1, 2016
  7. Jeff KingAug 1, 2016
  8. Junio C HamanoAug 1, 2016
  9. Duy NguyenAug 1, 2016
  10. Jeff KingAug 1, 2016
  11. Duy NguyenAug 1, 2016
  12. Jeff KingAug 1, 2016
  13. 2/2 pager: implement core.pagerEnv in configEric Wong, Aug 1, 2016
  14. Jeff KingAug 1, 2016
  15. 0/1 add PAGER_ENV to buildEric Wong, Aug 1, 2016
  16. 1/1 pager: move pager-specific setup into the buildEric Wong, Aug 1, 2016
  17. Junio C HamanoAug 1, 2016
  18. Jeff KingAug 1, 2016
  19. Junio C HamanoAug 2, 2016
  20. Eric WongAug 1, 2016
  21. Junio C HamanoAug 2, 2016
  22. Jeff KingAug 3, 2016
  23. Junio C HamanoAug 3, 2016
  24. Eric WongAug 3, 2016
  25. Junio C HamanoAug 3, 2016
  26. pager: move pager-specific setup into the buildEric Wong, Aug 4, 2016
  27. Jeff KingAug 4, 2016
  28. Eric WongAug 4, 2016
  29. Jeff KingAug 4, 2016
  30. pager: move pager-specific setup into the buildEric Wong, Aug 4, 2016
  31. Jeff KingAug 3, 2016
  32. Jeff KingAug 1, 2016

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.