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

Re: [PATCH v3] pager: move pager-specific setup into the build

From
Jeff King <peff@peff.net>
Date
Aug 4, 2016, 17:53 UTC
Message-ID
<20160804175328.tkjkoo34b43bvwsb@sigill.intra.peff.net>
In-Reply-To
<20160804113410.GA13908@starla>
On Thu, Aug 04, 2016 at 11:34:10AM +0000, Eric Wong wrote:
Show 15 quoted lines
> > > --- a/config.mak.uname
> > > +++ b/config.mak.uname
> > > @@ -209,6 +209,7 @@ ifeq ($(uname_S),FreeBSD)
> > >  	HAVE_PATHS_H = YesPlease
> > >  	GMTIME_UNRELIABLE_ERRORS = UnfortunatelyYes
> > >  	HAVE_BSD_SYSCTL = YesPlease
> > > +	PAGER_ENV = LESS=FRX LV=-c MORE=FRX
> > >  endif
> > 
> > Is it worth setting up PAGER_ENV's default values before including
> > config.mak.*, and then using "+=" here? That avoids this line getting
> > out of sync with the usual defaults.
> 
> Good point, but it makes ordering problematic for folks
> who want to override it config.mak or command-line.

I'm not sure it changes much for them. Their "=" in config.mak, etc, would override our default, and anything on the command line overrides all of the in-Makefile stuff anyway. The only difference would be if they use "+=" in config.mak, but there I think it would be an improvement.

I'm OK to leave it as-is until somebody actually cares, though.
Show 13 quoted lines
> > I know you said you don't like string parsing in C. Here is a patch (on
> > top of yours) that converts the parsing to shell, and generates a
> > pre-built array-of-struct (this is similar to the big series I posted
> > long ago, but just touching this one spot, not invading the whole
> > Makefile). Feel free to ignore it as over-engineered, but I thought I'd
> > throw it out there in case it appeals.
> 
> Yeah, but I'd rather not introduce more complexity into the
> build process, either (unless it's a performance-sensitive part,
> which this is not).  Also, while my original 2/2 to make it
> configurable at runtime was discarded, I wouldn't rule out
> somebody making a compelling case for it and it would be
> an easier change from the parse-at-runtime code.
Yeah, I had similar thoughts while writing it.
Your v4 patch looks fine to me.
-Peff
Previous: Eric WongNext: Eric Wong
Message 29 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.