Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

From: Jeff King <peff@peff.net>
Date: Jun 28, 2010, 09:40 UTC
Message-ID: <20100628094033.GA18629@coredump.intra.peff.net>
In-Reply-To: <20100626192203.GA19973@burratino>

On Sat, Jun 26, 2010 at 02:22:03PM -0500, Jonathan Nieder wrote:

Show 11 quoted lines

> Here’s a fix from the famous setup cleanup series[1].
> 
> The problem it fixes is somewhat obscure: ‘git -p foo’ is not paying
> attention to the repository-local ‘[core] pager’ configuration when
> run from a subdirectory.  But it is a real bug, and the fix gives
> an example of how to deal with repository setup and should be safe.
> 
> Patch is against master.  There is a small semantic conflict with
> jn/grep-open: SIMPLEPAGER should be changed to SIMPLEPAGERTTY in the
> prerequisites for the test_default_pager function.  Please let me
> know if I should push a merge commit to help resolve that.

I just read through the patches, and it seems like a reasonable fix. So in that sense, ACK.

But reading the message for patch 4/4, I can't help but wonder if the right way forward is for the git wrapper to _always_ find the repository as the very first thing. I.e., make finding the repository a non-destructive thing (especially don't end in a different directory), and just do it before we run any other code.

For a few commands like "init" or "clone", we would end up just ignoring this information, but the worst case should be a little bit of wasted effort.

And then all these silly setup and timing issues could just go away, as we would know that GIT_DIR and GIT_WORK_TREE would be set properly.

-Peff