Re: [PATCH] diff: setup pager only before diff contents truly ready

[Jeff King <peff@peff.net>]

On Tue, Dec 03, 2024 at 02:33:02PM +0900, Junio C Hamano wrote:

> >> It seems that this topic is waiting for a reroll?
> >
> > I think we could go either way. I outlined a few further possible steps,
> > but there is no need to hold up this first step. The only question is
> > whether or not to add a single test to show off and protect the
> > improvement.
> 
> Hmph, a few messages upthread
> <UZMh2lyzbLOgsf0PXfMnq6HnWVnCK3y36jY3IMKUykPi74ztNucf8bgywoeO0DdeApq31JDDGMZiEya99zAcI3l8y_zcVqiN8FpEnT1DRZU=@proton.me>
> (Ugh, why do some MUAs or mail providers use such an overly long
> message ID, Yuck) was where I got the impression that we were
> waiting for a reroll.

Yeah, I think Philip was offering to add some tests. Since he has been
quiet since, I do not have a strong opinion on whether we should just
take it as-is or let it go unless he comes back.

> I am not all that convinced that sprinkling setup_diff_pager() call
> all over the place is a good idea from longer-term maintainability's
> sake to begin with, by the way.  What problem are we really solving?
> Folks who run "git diff --no-such-option" see "behaviour inconsistency"?
> All I see is "error: invalid option: --invalid" followed by a help message,
> which is quite expected.

I think it is just about not starting the pager when there is no useful
output produced. Depending on your pager config, it is a little nicer if
we avoid it for a one-liner error. E.g., I do not use "-F", so "git diff
foo bar" drops me into the pager with a single error line.

-Peff