On Fri 03 May 2019 at 03:53, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Alex Branham <alex.branham@gmail.com>
>> Date: Wed, 24 Apr 2019 07:55:20 -0500
>
> Sorry for a long delay in responding.

And sorry for the also-delayed followup :-)

>> I've attached a patch that converts ediff to use lexical-binding. I've
>> been using it locally for a couple of weeks without noticing any issues,
>> though I'm not a super-heavy ediff user.
>
> I assume you ran all ediff tests we have, but did you also try
> commands outside the ediff-* group, to make sure they still work?  I
> think VC has some commands, and there's also emerge.

I checked out the VC commands. I'm not very familiar with smerge, but it
at least appears to function correctly from what I can tell. "make
check" passes too.

> I have a couple of questions regarding the changes:
>
>> (ediff-prepare-meta-buffer): Remove unused startup-hooks
>> (ediff-multi-patch-internal): Remove unused variable startup-hooks.
>
> startup-hooks are used by emerge and maybe by users.  Why remove it?

The attached patch keeps it now.

>> (ediff-date): Remove.
>
> Why?

It doesn't seem useful and people forget to modify it when they make
changes to the file.

>> @@ -714,9 +714,8 @@ behavior."
>>  ;; we may visit them recursively.  DIR1 is the directory to inspect.
>>  ;; MERGE-AUTOSTORE-DIR is the directory where to auto-store the results of
>>  ;; merges.  Can be nil.
>> -(defun ediff-get-directory-files-under-revision (jobname
>> -						 regexp dir1
>> -						 &optional merge-autostore-dir)
>> +(defun ediff-get-directory-files-under-revision (regexp dir1
>> +						        &optional merge-autostore-dir)
>
> This and other hunks change signatures of public functions, which is
> always a problem.  Is this a must?  Can't we leave the signatures
> alone?  If not, what are the problems that necessitate that?

Kept in the attached.

Thanks,
Alex