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

Re: [PATCH 2/2] config: handle conditional include when $GIT_DIR is not set up

From
Jeff King <peff@peff.net>
Date
Apr 18, 2017, 04:03 UTC
Message-ID
<20170418040305.t4soqhpllhd4f7u7@sigill.intra.peff.net>
In-Reply-To
<CACsJy8D-=3qPbEO-dq2+Mr-WZ+5RrivkSw9NX7_FhEJFy+59zA@mail.gmail.com>
On Mon, Apr 17, 2017 at 05:07:52PM +0700, Duy Nguyen wrote:
Show 18 quoted lines
> > You can see the second problem with:
> >
> >   # random external
> >   cat >git-foo <<-\EOF
> >   #!/bin/sh
> >   echo foo
> >   EOF
> >   chmod +x git-foo
> >   PATH=$PWD:$PATH
> >
> >   git init
> >   git config pager.foo 'sed s/^/repo:/'
> >   git -c pager.foo='sed s/^/cmdline:/' foo
> >
> > That command should prefer the cmdline config, but it doesn't.
> 
> I actually had problems seeing the problem, for some reason it didn't
> work for me. I guess I made a mistake somewhere.
I just double-checked it, and the above works for me. <shrug>
Show 8 quoted lines
> > The fix is something like what's below, which is easy on top of your new
> > options struct. I can wrap it up with a config message and test on top
> > of your series.
> 
> ... anyway I read this last sentence too late and spent too much time
> wrapping your changes in a patch (well most of the time was spent on
> writing a new test actually), so I'm  sending it out too. My test uses
> test-config though (I have given up on dealing with pager and tty).

Thanks for doing that. I had planned to do it myself to avoid making work for you, but what you posted looks good.

I hadn't thought about tty issues. You'd have to run it under test_terminal (which isn't even available everywhere). So your test-config version is probably the best bet.

Show 8 quoted lines
> Off topic. Back to the pager.foo thing (because I had to fight it a
> bit before giving up). I find it a bit unintuitive that "--paginate"
> forces the pager back to less (or $PAGER) when I say "pager.foo =
> my-favorite-pager". Back when pager.foo is a boolean thing, it makes
> sense for --paginate to override the "to page or not to page"
> decision. But then you added a new meaning too pager.foo (what command
> to run). "--paginate" should respect the command pager.foo specifies
> when its value is a command, I think.

I never noticed that before, and I agree that it's nonsense. Using "-p" should be unnecessary if you have pager.foo defined (because it already implies "yes, use the pager"). But if you were to do it anyway, I agree it should not ignore the configured pager. One might use an extra "-p" hoping that it would override the tty check, but it doesn't. It's too late to change that now, I think, but I have been tempted to add a --paginate-me-harder option. It would be handy for tests.

-Peff
Previous: Junio C HamanoNext: Jeff King
Message 22 of 24 in “includeIf breaks calling dashed externals”
  1. Bert WesargApr 14, 2017
  2. Jeff KingApr 14, 2017
  3. Duy NguyenApr 15, 2017
  4. Jeff KingApr 16, 2017
  5. 1/2 config: prepare to pass more info in git_config_with_options()Nguyễn Thái Ngọc Duy, Apr 16, 2017
  6. 2/2 config: handle conditional include when $GIT_DIR is not set upNguyễn Thái Ngọc Duy, Apr 16, 2017
  7. Jeff KingApr 16, 2017
  8. Duy NguyenApr 17, 2017
  9. Jeff KingApr 18, 2017
  10. Duy NguyenApr 17, 2017
  11. 1/3 config: prepare to pass more info in git_config_with_options()Nguyễn Thái Ngọc Duy, Apr 17, 2017
  12. 2/3 config: handle conditional include when $GIT_DIR is not set upNguyễn Thái Ngọc Duy, Apr 17, 2017
  13. Junio C HamanoApr 18, 2017
  14. Junio C HamanoApr 18, 2017
  15. Jeff KingApr 18, 2017
  16. 3/3 config: correct file reading order in read_early_config()Nguyễn Thái Ngọc Duy, Apr 17, 2017
  17. Jeff KingApr 18, 2017
  18. Junio C HamanoApr 18, 2017
  19. Jeff KingApr 18, 2017
  20. Junio C HamanoApr 18, 2017
  21. Junio C HamanoApr 18, 2017
  22. Jeff KingApr 18, 2017
  23. Jeff KingApr 16, 2017
  24. Duy NguyenApr 17, 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.