Re: [PATCH 1/2] am: pay attention to user-defined context size

From: Elijah Newren <newren@gmail.com>
Date: Dec 20, 2019, 22:22 UTC
Message-ID: <CABPp-BGvh-NvooVE-K-WYpv9RAs4BZ_d0=Mv=b5iEoGbNt5amA@mail.gmail.com>
In-Reply-To: <CABPp-BFYyF5RbZVuwo8LtDZZQ9KQ2od4FQqQYaeSqNCNKcZXPw@mail.gmail.com>

Hi,

On Fri, Dec 20, 2019 at 11:38 AM Elijah Newren <newren@gmail.com> wrote:

Show 10 quoted lines

>
> On Fri, Dec 20, 2019 at 11:28 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Besides, I do not think it should be affected by any diff_*UI*_config()
> > in the first place.
>
> Does that mean that diff.context is checked in the wrong place in
> diff.c, and should be moved from git_diff_ui_config() to
> git_diff_basic_config()?  (And perhaps the same is true for
> diff.algorithm?)

So, referring to git_diff_ui_config() as ui and git_diff_basic_config() as basic:

* Moving diff.algorithm from ui to basic requires updating the error
message printed by t3701.47
* Moving diff.context from ui to basic breaks t4055.6 (which wants
diff.context to NOT affect plumbing)
* Moving diff.suppressblankempty from basic to ui causes no issues
* Moving diff.color.* and color.diff.* and diff.*.* from basic to ui
causes no issues, but you have to move the userdiff_config if you want
to move the color config or else t4020.12 breaks.

In particular, t4055.6 is pretty interesting in that it was a test specifically created for the sole purpose of declaring that diff.context should NOT affect plumbing. So if it's not plumbing by that test, and it's not *UI* as per what you say, what exactly is it? Do I just make am directly check diff.context and ignore any other diff settings?

Not sure where to go on this...