Re: [PATCH 0/9] built-in add -p: add support for the same config settings as the Perl version

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 25, 2019, 12:09 UTC
Message-ID: <nycvar.QRO.7.76.6.1912251303320.46@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqimm5601h.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Tue, 24 Dec 2019, Junio C Hamano wrote:

Show 34 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
> > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >
> >> base-commit: 2d4b85ddc76af3e703e6e3a6a72319b5e79c2d8b
> >
> > It is not generally helpful to those who reads this list to use a
> > commit that is not part of history leading to my 'pu' or 'next' as
> > the base.
>
> I think there was only one spot that needed adjusting to the newer
> iteration of the js/patch-mode-in-others-in-c series.
>
> This may have started as "there are some configuration variables
> that are ignored in the C version, fix them" and that may be why
> the pull-request branch says "config-settings", but overall, I think
> the bulk of the change ends up being a "how would we implement the
> annoying-to-implement-portably single-key behaviour".
>
> I think it is a mistake to write the lower-level terminal access
> code without using established libraries (or write it with a higher
> level abstraction offered by scripting languages like Perl and
> Pythnon), and I would personally take, given a choice between
> accepting such maintenance/porting liability and dropping of
> single-key behaviour, the latter in any second.
>
> I wonder if it makes sense to split this series into two so that the
> early and easier part for leftover config bits can graduate
> separately early in the next cycle, instead of letting the parts
> that tackles the terminal nightmare (note that the problem being
> nightmare is not the fault of this topic) which would inevitably
> take more time to stabilize take the remainder of the series hostage
> to it.

I wondered about the same two things: whether to use an established library, and whether to split off the patches for the config settings.

Alas, I did not find any established library that I could use on Windows, so there is _already_ a precedent for doing it the way my patches do it. And if we already have to e.g. spawn `infocmp` and poll to catch Escape sequences for Windows, my reasoning went: why not just do it for all platforms? This simplifies the overall complexity of the patches, as we do not have to do _too_ different things for Windows vs non-Windows.

About the config settings, sure, they could be split off, but for me this patch series really is about getting the built-in to reach parity with the Perl script version of `git add -i`/`git add -p`.

In short, I really would like to keep the overall direction of this patch series intact.

Ciao, Dscho