GNU bug report logs - #35413
[PATCH] Use lexical binding for ediff

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 35413 in the body.
You can then email your comments to 35413 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <alex.branham <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Use lexical binding for ediff
Date: Wed, 24 Apr 2019 07:55:20 -0500

[Message part 1 (text/plain, inline)]

Hello -

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.

Thanks,
Alex

[0001-Use-lexical-binding-for-ediff.patch (text/x-patch, attachment)]

Message #8 received at 35413 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: 35413 <at> debbugs.gnu.org
Subject: Re: bug#35413: [PATCH] Use lexical binding for ediff
Date: Fri, 03 May 2019 11:53:05 +0300

> From: Alex Branham <alex.branham <at> gmail.com>
> Date: Wed, 24 Apr 2019 07:55:20 -0500

Sorry for a long delay in responding.

> 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 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?

> (ediff-date): Remove.

Why?

> @@ -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?

Thanks.

Message #11 received at 35413 <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <alex.branham <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 35413 <at> debbugs.gnu.org
Subject: Re: bug#35413: [PATCH] Use lexical binding for ediff
Date: Tue, 14 May 2019 07:31:23 -0500

[Message part 1 (text/plain, inline)]

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

>> From: Alex Branham <alex.branham <at> 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

[0001-Use-lexical-binding-for-ediff.patch (text/x-patch, inline)]

From c7138250126c1ada210ed4e89df61c041a372484 Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham <at> 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 <at> 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 <at> 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

Message #14 received at 35413 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: 35413 <at> debbugs.gnu.org
Subject: Re: bug#35413: [PATCH] Use lexical binding for ediff
Date: Fri, 07 Jun 2019 17:24:53 +0300

> From: Alex Branham <alex.branham <at> gmail.com>
> Cc: 35413 <at> debbugs.gnu.org
> Date: Tue, 14 May 2019 07:31:23 -0500
> 
> On Fri 03 May 2019 at 03:53, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
> >> From: Alex Branham <alex.branham <at> 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 :-)

Sorry for another long delay, this version is fine with me.

Message #19 received at 35413-done <at> debbugs.gnu.org (full text, mbox):

From: Alex Branham <alex.branham <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 35413-done <at> debbugs.gnu.org
Subject: Re: bug#35413: [PATCH] Use lexical binding for ediff
Date: Sun, 09 Jun 2019 10:02:08 -0500

On Fri 07 Jun 2019 at 09:24, Eli Zaretskii <eliz <at> gnu.org> wrote:

> Sorry for another long delay, this version is fine with me.

Great, pushed as 963d4e24263b0ff2add1a223f00387ca53d0658f to appear in Emacs 27.

Thanks,
Alex

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.