GNU bug report logs - #26378
26.0.50; Hitting 'n' during ediff gives Error

Previous Next

Full log

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

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: npostavs <at> users.sourceforge.net
Cc: phst <at> google.com, 26378 <at> debbugs.gnu.org
Subject: Re: bug#26378: 26.0.50; Hitting 'n' during ediff gives Error
Date: Thu, 06 Apr 2017 12:23:42 +0000

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

Hi Noam,

Thanks for the comments.

How does this look?

From 99e290cec79754d8d92ec6dcf3c58594782a677b Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi <at> gmail.com>
Date: Wed, 5 Apr 2017 17:16:33 -0400
Subject: [PATCH] Check that file argument is a string

* lisp/vc/ediff-diff.el (ediff-exec-process): Check that the argument
  passed to `file-local-copy' is a string (Bug#26378).  Also fix
  the existing comment for this function, and convert it to its
  doc-string.
---
 lisp/vc/ediff-diff.el | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/lisp/vc/ediff-diff.el b/lisp/vc/ediff-diff.el
index cfa08ef360..3b2a85afdd 100644
--- a/lisp/vc/ediff-diff.el
+++ b/lisp/vc/ediff-diff.el
@@ -1134,12 +1134,22 @@ ediff-setup-diff-regions3
    ))


-;; Execute PROGRAM asynchronously, unless OS/2, Windows-*, or DOS, or
unless
-;; SYNCH is non-nil.  BUFFER must be a buffer object, and must be alive.
The
-;; OPTIONS arg is a list of options to pass to PROGRAM. It may be a blank
-;; string.  All elements in FILES must be strings.  We also delete nil from
-;; args.
 (defun ediff-exec-process (program buffer synch options &rest files)
+  "Execute the diff PROGRAM.
+
+The PROGRAM output is sent to BUFFER, which must be a live buffer
+object.
+
+The PROGRAM is executed asynchronously unless `system-type' is
+`windows-nt' or `ms-dos', or unless SYNCH is non-nil.
+
+OPTIONS is a string of space-separated options to pass to PROGRAM.  It
+may be a blank string.
+
+FILES is a list of filenames to pass to PROGRAM.  This list may
+contain a nil element too.
+
+nil and \"\" elements in OPTIONS and FILES are ignored."
   (let ((data (match-data))
  ;; If this is a buffer job, we are diffing temporary files
  ;; produced by Emacs with ediff-coding-system-for-write, so
@@ -1151,8 +1161,9 @@ ediff-exec-process
  args)
     (setq args (append (split-string options)
                        (mapcar (lambda (file)
-                                 (file-name-unquote
-                                  (or (file-local-copy file) file)))
+                                 (when (stringp file)
+                                   (file-name-unquote
+                                    (or (file-local-copy file) file))))
                                files)))
     (setq args (delete "" (delq nil args))) ; delete nil and "" from
arguments
     ;; the --binary option, if present, should be used only for buffer jobs
-- 
2.11.0


On Wed, Apr 5, 2017 at 7:21 PM <npostavs <at> users.sourceforge.net> wrote:

> Now that we're looking at the whole thing, I have a few more comments.
>
> Kaushal Modi <kaushal.modi <at> gmail.com> writes:
>
> > +  "Execute the diff PROGRAM.
> > +
> > +The PROGRAM output is sent to BUFFER, which must be a buffer object,
> > +and must be alive.
>
> It would flow a bit better to say "which must be a live buffer object".
>

I agree.


> > +
> > +The PROGRAM is executed asynchronously unless the OS is OS/2,
> > +Windows-*, or DOS, or unless SYNCH is non-nil.
>
> I don't see any reference to OS/2 in the code, so I think we should just
> drop it.


Makes sense.


> And I'm not sure what the "-*" in "Windows-*" means;


It means Windows-95, Windows NT, Windows 8, Windows 10, etc.


> I think
> we should just say "unless `system-type' is `windows-nt' or `ms-dos'".
>

I am fine with that.


> > +
> > +OPTIONS is a list of options to pass to PROGRAM.  It may be a blank
> > +string.
>
> This seems to be wrong, OPTIONS is not a list.  It should say "OPTIONS
> is a string of space-separated options to pass to PROGRAM".
>

You are right; making that change.


> > +
> > +An element in FILES must be either a string or nil.
> > +
> > +We also delete nil and \"\" from all arguments."
>
> I think these last 2 sentences should be combined into something like:
>
> "FILES is a list of filenames to pass to PROGRAM; nil and \"\" elements
> are ignored."
>

I have tweaked this part slightly (see patch in this email). Based on the
code, nil and "" elements will be removed from a concatenated list of
OPTIONS and FILES; not just FILES.
-- 

Kaushal Modi

[Message part 2 (text/html, inline)]

Previous Next

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