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

[Kaartic Sivaraam <kaartic.sivaraam@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:
>>> ### 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