Re: [GSoC][RFC] Convert mergetool to builtin

From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Mar 19, 2020, 08:42 UTC
Message-ID: <1bb8e4f9-6e6e-0874-b1ef-47eccf93a98b@gmail.com>
In-Reply-To: <CAHk66fu7dZ4H8tvnbxfdBRcRdx-3f_cJSdVAoKrE3UbR3nehfA@mail.gmail.com>

Hi Abishek,

Just wanted to share a general suggestion which is not related to your proposal. In future, when you quote a portion of the e-mail retain the complete meta information about the quote (like the one you see below) or the person who wrote the quoted portion at the very least. It helps people who join late in the discussion to *quickly* get up-to-speed with the discussion.

Anyways, good luck with your proposal. :)

On 18-03-2020 22:00, Abhishek Kumar wrote:

Show 37 quoted lines

>>> ### Conversion of mergetool--lib
>>>
>>> As mentioned earlier, conversion of the mergetool-related scripts has to be
>>> spread over 2-3 SoC or similar projects due to the size of scripts involved.
>>> Conversion of mergetool would set up most of the plumbing required for
>>> mergetool--lib and makes the subsequent conversion possible.
>>
>> I wonder if it would be better to convert git-mergetool--lib.sh first
>> and then git-difftool--helper.sh and git-mergetool.sh that are using
>> it.
> 
> I had been agonizing over this decision while I was initially writing
> the proposal.
> 
> My justifications for mergetool.sh over mergetool--lib.sh at the time were:
> 
> 1. mergetool.sh makes many more calls to git subcommands than mergetool--lib.
> Therefore, its performance improves from both moving from bash to C and use of
> git internals.
> 2. I had *incorrectly* counted overall lines to be over 1,700 with
> 1,200 lines for mergetool--lib + difftool--helper + mergetools/ whereas it
> actually stands at rather manageable 1,000 lines with mergetools/ being fairly
> formulaic.
> 
> There are solid reasons to consider the conversion of mergetool--lib too:
> 1. The code path of difftool-helper would be entirely in C, improving its
> performance on Windows particularly well.
> 2. It has two well-defined entry points, which makes conversion straightforward
> and with less code churn.
> 3. It could be done with the more frequently-adopted approach of script
> calling the builtin.
> 
> As it stands now, I am open to converting either scripts.
> 
> I have CC'ed Johannes as well. I am sure he would like to weigh in
> this discussion.
>

-- Sivaraam