On Mon, Apr 17, 2017 at 05:07:52PM +0700, Duy Nguyen wrote: > > 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> > > 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. > 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