Re: [PATCH v2] Enable auto-merge for meld to follow the vim-diff beharior

From: David Aguilar <davvid@gmail.com>
Date: Jun 30, 2020, 06:48 UTC
Message-ID: <20200630064800.GB1962986@gmail.com>
In-Reply-To: <bfc401d64df4$cd189e70$6749db50$@zoom.us>

On Mon, Jun 29, 2020 at 05:08:02PM +0800, lin.sun@zoom.us wrote:

Show 15 quoted lines

> Hi Danh,
> 
> > It seems like only David Aguilar (Cc-ed) works on that file.
> >Look into  b12d04503b (mergetools/meld: make usage of `--output` configurable and more robust, 2014-10-15), it looks like we need to check if --auto-merge option is available in meld.
> >Someone still live with the ancient tools ;).
> >meld has known --output for a long time but we still have a check for it.
> 
> Thank you for your hints, I changed the patch for checking the option "--auto-merge" in meld and use this option only if it's available.
> The last patch is appended in attachment, or https://github.com/git/git/commit/3b70fd0bfc4086a08e27c869ff7492d567e8fdc2
> 
> Look forward it can be merged this time. 
> Thanks
> 
> Regards
> Lin

Hello, thank you for making this change and keeping the checks around for the older versions.

One comment on the patch -- it looks like it duplicates the version check logic. Would it be possible to coalesce the version check so that we only exec meld once for the two version checks?

The changes look good nonetheless, and perhaps combining the checks would make it too complex, so this looks good to me.

Can you please submit the patch to the list here? Once you do, please feel free to add my sign-off:

Signed-off-by: David Aguilar <davvid@gmail.com>

cheers,

-- 
David