bug#35413: [PATCH] Use lexical binding for ediff

[Alex Branham]

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

>From c7138250126c1ada210ed4e89df61c041a372484 Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham@gmail.com>
Date: Thu, 9 May 2019 07:47:26 -0500
Subject: [PATCH] Use lexical binding for ediff

* lisp/vc/ediff-diff.el:
* lisp/vc/ediff-help.el:
* lisp/vc/ediff-hook.el:
* lisp/vc/ediff-init.el:
* lisp/vc/ediff-merg.el:
* lisp/vc/ediff-vers.el:
* lisp/vc/ediff-wind.el:
* lisp/vc/ediff-mult.el:
* lisp/vc/ediff-ptch.el:
* lisp/vc/ediff.el: Use lexical binding.
(ediff-version): Increase.
(ediff-date): Remove.
---
 lisp/vc/ediff-diff.el |  2 +-
 lisp/vc/ediff-help.el |  3 ++-
 lisp/vc/ediff-hook.el |  2 +-
 lisp/vc/ediff-init.el |  2 +-
 lisp/vc/ediff-merg.el |  2 +-
 lisp/vc/ediff-mult.el | 13 ++++++-------
 lisp/vc/ediff-ptch.el |  2 +-
 lisp/vc/ediff-vers.el |  2 +-
 lisp/vc/ediff-wind.el |  2 +-
 lisp/vc/ediff.el      | 11 ++++-------
 10 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/lisp/vc/ediff-diff.el b/lisp/vc/ediff-diff.el
index a1d27af79d..f6b68bbd7d 100644
--- a/lisp/vc/ediff-diff.el
+++ b/lisp/vc/ediff-diff.el
@@ -1,4 +1,4 @@
-;;; ediff-diff.el --- diff-related utilities
+;;; ediff-diff.el --- diff-related utilities  -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 1994-2019 Free Software Foundation, Inc.
 
diff --git a/lisp/vc/ediff-help.el b/lisp/vc/ediff-help.el
index 11c8b35bca..74a4068a7f 100644
--- a/lisp/vc/ediff-help.el
+++ b/lisp/vc/ediff-help.el
@@ -1,4 +1,4 @@
-;;; ediff-help.el --- Code related to the contents of Ediff help buffers
+;;; ediff-help.el --- Code related to the contents of Ediff help buffers  -*- 
lexical-binding: t; -*-
 
 ;; Copyright (C) 1996-2019 Free Software Foundation, Inc.
 
@@ -30,6 +30,7 @@ ediff-multiframe
 ;; end pacifier
 
 (require 'ediff-init)
+(defvar ediff-multiframe)
 
 ;; Help messages
 
diff --git a/lisp/vc/ediff-hook.el b/lisp/vc/ediff-hook.el
index 84122150ad..6ece7af5e6 100644
--- a/lisp/vc/ediff-hook.el
+++ b/lisp/vc/ediff-hook.el
@@ -1,4 +1,4 @@
-;;; ediff-hook.el --- setup for Ediff's menus and autoloads
+;;; ediff-hook.el --- setup for Ediff's menus and autoloads  -*- 
lexical-binding: t; -*-
 
 ;; Copyright (C) 1995-2019 Free Software Foundation, Inc.
 
diff --git a/lisp/vc/ediff-init.el b/lisp/vc/ediff-init.el
index a74d6a8b4d..41871d4b7c 100644
--- a/lisp/vc/ediff-init.el
+++ b/lisp/vc/ediff-init.el
@@ -1,4 +1,4 @@
-;;; ediff-init.el --- Macros, variables, and defsubsts used by Ediff
+;;; ediff-init.el --- Macros, variables, and defsubsts used by Ediff  -*- 
lexical-binding: t; -*-
 
 ;; Copyright (C) 1994-2019 Free Software Foundation, Inc.
 
diff --git a/lisp/vc/ediff-merg.el b/lisp/vc/ediff-merg.el
index a511f4488f..e08d899bd0 100644
--- a/lisp/vc/ediff-merg.el
+++ b/lisp/vc/ediff-merg.el
@@ -1,4 +1,4 @@
-;;; ediff-merg.el --- merging utilities
+;;; ediff-merg.el --- merging utilities  -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 1994-2019 Free Software Foundation, Inc.
 
diff --git a/lisp/vc/ediff-mult.el b/lisp/vc/ediff-mult.el
index 21f89168b3..8ac8eff986 100644
--- a/lisp/vc/ediff-mult.el
+++ b/lisp/vc/ediff-mult.el
@@ -1,4 +1,4 @@
-;;; ediff-mult.el --- support for multi-file/multi-buffer processing in Ediff
+;;; ediff-mult.el --- support for multi-file/multi-buffer processing in Ediff  
-*- lexical-binding: t; -*-
 
 ;; Copyright (C) 1995-2019 Free Software Foundation, Inc.
 
@@ -714,7 +714,7 @@ ediff-intersect-directories
 ;; 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
+(defun ediff-get-directory-files-under-revision (_jobname
                                                 regexp dir1
                                                 &optional merge-autostore-dir)
   (let (lis1 elt common auxdir1)
@@ -760,7 +760,7 @@ ediff-get-directory-files-under-revision
                                      auxdir1 nil nil
                                      merge-autostore-dir nil)
      (mapcar (lambda (elt) (ediff-make-new-meta-list-element
-                           (expand-file-name (concat auxdir1 elt)) nil nil))
+                      (expand-file-name (concat auxdir1 elt)) nil nil))
             common))
     ))
 
@@ -798,8 +798,8 @@ ediff-get-directory-files-under-revision
 ;; Prepare meta-buffer in accordance with the argument-function and
 ;; redraw-function.  Must return the created  meta-buffer.
 (defun ediff-prepare-meta-buffer (action-func meta-list
-                                 meta-buffer-name redraw-function
-                                 jobname &optional startup-hooks)
+                                             meta-buffer-name redraw-function
+                                             jobname &optional _startup-hooks)
   (let* ((meta-buffer-name
          (ediff-unique-buffer-name meta-buffer-name "*"))
         (meta-buffer (get-buffer-create meta-buffer-name)))
@@ -1583,8 +1583,7 @@ ediff-mark-for-hiding-at-pos
 
 ;; Returns whether session was marked or unmarked
 (defun ediff-mark-session-for-hiding (info unmark)
-  (let ((session-buf (ediff-get-session-buffer info))
-       ignore)
+  (let (ignore)
     (cond ((eq unmark 'mark) (setq unmark nil))
          ((eq (ediff-get-session-status info) ?H) (setq unmark t))
          (unmark  ; says unmark, but the marker is different from H
diff --git a/lisp/vc/ediff-ptch.el b/lisp/vc/ediff-ptch.el
index 70c03b5c01..21e1e2ee16 100644
--- a/lisp/vc/ediff-ptch.el
+++ b/lisp/vc/ediff-ptch.el
@@ -1,4 +1,4 @@
-;;; ediff-ptch.el --- Ediff's  patch support
+;;; ediff-ptch.el --- Ediff's  patch support  -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 1996-2019 Free Software Foundation, Inc.
 
diff --git a/lisp/vc/ediff-vers.el b/lisp/vc/ediff-vers.el
index 664ae5ae94..049fdc880b 100644
--- a/lisp/vc/ediff-vers.el
+++ b/lisp/vc/ediff-vers.el
@@ -1,4 +1,4 @@
-;;; ediff-vers.el --- version control interface to Ediff
+;;; ediff-vers.el --- version control interface to Ediff  -*- lexical-binding: 
t; -*-
 
 ;; Copyright (C) 1995-1997, 2001-2019 Free Software Foundation, Inc.
 
diff --git a/lisp/vc/ediff-wind.el b/lisp/vc/ediff-wind.el
index 492ddd3417..559a20b023 100644
--- a/lisp/vc/ediff-wind.el
+++ b/lisp/vc/ediff-wind.el
@@ -1,4 +1,4 @@
-;;; ediff-wind.el --- window manipulation utilities
+;;; ediff-wind.el --- window manipulation utilities  -*- lexical-binding: t; 
-*-
 
 ;; Copyright (C) 1994-1997, 2000-2019 Free Software Foundation, Inc.
 
diff --git a/lisp/vc/ediff.el b/lisp/vc/ediff.el
index 0dfbe2ea66..e13c7b93a9 100644
--- a/lisp/vc/ediff.el
+++ b/lisp/vc/ediff.el
@@ -1,21 +1,18 @@
-;;; ediff.el --- a comprehensive visual interface to diff & patch
+;;; ediff.el --- a comprehensive visual interface to diff & patch  -*- 
lexical-binding: t; -*-
 
 ;; Copyright (C) 1994-2019 Free Software Foundation, Inc.
 
 ;; Author: Michael Kifer <kifer@cs.stonybrook.edu>
 ;; Created: February 2, 1994
 ;; Keywords: comparing, merging, patching, vc, tools, unix
-;; Version: 2.81.4
+;; Version: 2.81.6
+(defconst ediff-version "2.81.6" "The current version of Ediff")
 
 ;; Yoni Rabkin <yoni@rabkins.net> contacted the maintainer of this
 ;; file on 20/3/2008, and the maintainer agreed that when a bug is
 ;; filed in the Emacs bug reporting system against this file, a copy
 ;; of the bug report be sent to the maintainer's email address.
 
-(defconst ediff-version "2.81.5" "The current version of Ediff")
-(defconst ediff-date "July 4, 2013" "Date of last update")
-
-
 ;; This file is part of GNU Emacs.
 
 ;; GNU Emacs is free software: you can redistribute it and/or modify
@@ -1546,7 +1543,7 @@ ediff-version
           (interactive-p)
         (called-interactively-p 'interactive))
       (message "%s" (ediff-version))
-    (format "Ediff %s of %s" ediff-version ediff-date)))
+    (format "Ediff %s" ediff-version)))
 
 ;; info is run first, and will autoload info.el.
 (declare-function Info-goto-node "info" (nodename &optional fork strict-case))
-- 
2.21.0