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

Previous Next

Package: emacs;

Reported by: Kaushal Modi <kaushal.modi <at> gmail.com>

Date: Wed, 5 Apr 2017 20:58:02 UTC

Severity: normal

Tags: fixed

Merged with 26385, 26394

Found in version 26.0.50

Done: npostavs <at> users.sourceforge.net

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Wed, 05 Apr 2017 20:58:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Kaushal Modi <kaushal.modi <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Wed, 05 Apr 2017 20:58:02 GMT) Full text and rfc822 format available.

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

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 26.0.50; Hitting 'n' during ediff gives Error
Date: Wed, 05 Apr 2017 20:57:21 +0000
[Message part 1 (text/plain, inline)]
Recipe to create the error:

- emacs -Q
- Create two buffers; I am calling them a and b: C-x b a, C-x b b
- Put something in each buffer to diff.. I put "abc" and "abcd"
- M-x ediff-buffers, and select those two buffers
- Hit 'n' key to go to the first line showing the difference.

You would then get:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  find-file-name-handler(nil file-local-copy)
  file-local-copy(nil)
  #[(file) "\301 !\206
  mapcar(#[(file) "\301 !\206
  ediff-exec-process("diff" #<buffer *ediff-fine-diff<2>*> synchronize ""
"/tmp/fineDiffA15751M4K" "/tmp/fineDiffB15751ZCR" nil)
  ediff-setup-fine-diff-regions("/tmp/fineDiffA15751M4K"
"/tmp/fineDiffB15751ZCR" nil 0)
  ediff-make-fine-diffs(0 noforce)
  ediff-install-fine-diff-if-necessary(0)
  ediff-next-difference(1)
  funcall-interactively(ediff-next-difference 1)
  call-interactively(ediff-next-difference nil nil)
  command-execute(ediff-next-difference)

[image: pasted1]

In GNU Emacs 26.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 2.24.23)
 of 2017-04-05 built on ...
Repository revision: f1d34d9136fbf1dc2cf58b5ba36311451f024956
Windowing system distributor 'The X.Org Foundation', version 11.0.60900000
System Description: Red Hat Enterprise Linux Workstation release 6.6
(Santiago)

Recent messages:
file-local-copy: Wrong type argument: stringp, nil
Debug on Error enabled globally
Debug on Error disabled globally
Debug on Error enabled globally
Computing differences between ediff15751Nk1 and ediff15751_tE ...
Buffer A: Processing difference region 0 of 1
Buffer B: Processing difference region 0 of 1
Processing difference regions ... done
Refining difference region 1 ...
Entering debugger...

Configured using:
 'configure --with-modules
 --prefix=/home/kmodi/usr_local/apps/6/emacs/master
 '--program-transform-name=s/^ctags$/ctags_emacs/'
 'CPPFLAGS=-fgnu89-inline -I/home/kmodi/usr_local/6/include
 -I/usr/include/freetype2 -I/usr/include' 'CFLAGS=-ggdb3 -O0'
 'CXXFLAGS=-ggdb3 -O0' 'LDFLAGS=-L/home/kmodi/usr_local/6/lib
 -L/home/kmodi/usr_local/6/lib64 -ggdb3'
 PKG_CONFIG_PATH=/home/kmodi/usr_local/6/lib/pkgconfig:/home/kmodi/usr_local/6/lib64/pkgconfig:/cad/adi/apps/gnu/linux/x86_64/6/lib/pkgconfig:/cad/adi/apps/gnu/linux/x86_64/6/lib64/pkgconfig:/usr/lib/pkgconfig:/usr/lib64/pkgconfig:/usr/share/pkgconfig:/lib/pkgconfig:/lib64/pkgconfig'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK2 X11 MODULES

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=none
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message puny seq byte-opt subr-x gv
bytecomp byte-compile cl-extra cconv dired dired-loaddefs format-spec
rfc822 mml mml-sec password-cache epa derived epg epg-config gnus-util
rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils help-mode easymenu debug cus-start
cus-load ediff-merg ediff-wind ediff-diff ediff-mult ediff-help
ediff-init cl-loaddefs pcase cl-lib ediff-util ediff time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript case-table epa-hook jka-cmpr-hook help
simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs button
faces cus-face macroexp files text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 114425 9298)
 (symbols 48 22242 1)
 (miscs 40 143 141)
 (strings 32 22533 4406)
 (string-bytes 1 740306)
 (vectors 16 16276)
 (vector-slots 8 506737 6409)
 (floats 8 72 54)
 (intervals 56 433 0)
 (buffers 976 25)
 (heap 1024 45428 673))

-- 

Kaushal Modi
[Message part 2 (text/html, inline)]
[pasted1 (image/png, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Wed, 05 Apr 2017 21:04:02 GMT) Full text and rfc822 format available.

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

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: 26378 <at> debbugs.gnu.org, phst <at> google.com
Subject: 26.0.50; Hitting 'n' during ediff gives Error
Date: Wed, 05 Apr 2017 21:03:43 +0000
[Message part 1 (text/plain, inline)]
Reverting this commit fixes this bug.

http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=f4b50dad8d5eade04f495c693c0bca46060b25cb
-- 

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

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Wed, 05 Apr 2017 21:22:01 GMT) Full text and rfc822 format available.

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

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: 26378 <at> debbugs.gnu.org, phst <at> google.com
Subject: Re: 26.0.50; Hitting 'n' during ediff gives Error
Date: Wed, 05 Apr 2017 21:20:56 +0000
[Message part 1 (text/plain, inline)]
This patch fixes the error:

From 9de1cad8697781ac9a3729087cd38f1da7374ad4 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)
---
 lisp/vc/ediff-diff.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/vc/ediff-diff.el b/lisp/vc/ediff-diff.el
index cfa08ef360..79db0dc042 100644
--- a/lisp/vc/ediff-diff.el
+++ b/lisp/vc/ediff-diff.el
@@ -1151,8 +1151,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

Can you please review and commit this if alright?

Thanks.

> --

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

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Wed, 05 Apr 2017 21:54:02 GMT) Full text and rfc822 format available.

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

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: phst <at> google.com, 26378 <at> debbugs.gnu.org
Subject: Re: bug#26378: 26.0.50; Hitting 'n' during ediff gives Error
Date: Wed, 5 Apr 2017 17:53:51 -0400
On Wed, Apr 5, 2017 at 5:20 PM, Kaushal Modi <kaushal.modi <at> gmail.com> wrote:
> Subject: [PATCH] Check that file argument is a string

>                         (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)))


We should probably update the comment above `ediff-exec-process' where it says:

    All elements in FILES must be strings.

This seems to be a lie, as they can also be nil.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Wed, 05 Apr 2017 22:37:02 GMT) Full text and rfc822 format available.

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

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: Noam Postavsky <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: Wed, 05 Apr 2017 22:36:03 +0000
[Message part 1 (text/plain, inline)]
On Wed, Apr 5, 2017 at 5:53 PM Noam Postavsky <
npostavs <at> users.sourceforge.net> wrote:

> On Wed, Apr 5, 2017 at 5:20 PM, Kaushal Modi <kaushal.modi <at> gmail.com>
> wrote:
> > Subject: [PATCH] Check that file argument is a string
>
> We should probably update the comment above `ediff-exec-process' where it
> says:
>
>     All elements in FILES must be strings.
>
> This seems to be a lie, as they can also be nil.
>

How does this look?

From d4c732d16a5a79293c6f205ba9c75f82b17b3aa8 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 convert
  an existing comment for the function to its doc-string.
---
 lisp/vc/ediff-diff.el | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/lisp/vc/ediff-diff.el b/lisp/vc/ediff-diff.el
index cfa08ef360..ca7c3ab517 100644
--- a/lisp/vc/ediff-diff.el
+++ b/lisp/vc/ediff-diff.el
@@ -1134,12 +1134,21 @@ 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 buffer object,
+and must be alive.
+
+The PROGRAM is executed asynchronously unless the OS is OS/2,
+Windows-*, or DOS, or unless SYNCH is non-nil.
+
+OPTIONS is a list of options to pass to PROGRAM.  It may be a blank
+string.
+
+An element in FILES must be either a string or nil.
+
+We also delete nil and \"\" from all arguments."
   (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 +1160,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




-- 

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

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Wed, 05 Apr 2017 23:22:02 GMT) Full text and rfc822 format available.

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

From: npostavs <at> users.sourceforge.net
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: phst <at> google.com, 26378 <at> debbugs.gnu.org
Subject: Re: bug#26378: 26.0.50; Hitting 'n' during ediff gives Error
Date: Wed, 05 Apr 2017 19:23:14 -0400
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".

> +
> +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.  And I'm not sure what the "-*" in "Windows-*" means; I think
we should just say "unless `system-type' is `windows-nt' or `ms-dos'".

> +
> +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".

> +
> +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."




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Thu, 06 Apr 2017 12:25:02 GMT) Full text and rfc822 format available.

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)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Thu, 06 Apr 2017 12:45:01 GMT) Full text and rfc822 format available.

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

From: npostavs <at> users.sourceforge.net
To: Kaushal Modi <kaushal.modi <at> gmail.com>
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 08:45:37 -0400
Kaushal Modi <kaushal.modi <at> gmail.com> writes:

> +
> +The PROGRAM is executed asynchronously unless `system-type' is
> +`windows-nt' or `ms-dos', or unless SYNCH is non-nil.

I think the second "unless" is redundant.

> +
> +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."

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

But OPTIONS can't have nil or "" in it (or any "elements", really), so I
don't think there is any need to talk about that in the docstring.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Thu, 06 Apr 2017 14:38:01 GMT) Full text and rfc822 format available.

Message #29 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 14:36:58 +0000
[Message part 1 (text/plain, inline)]
On Thu, Apr 6, 2017 at 8:44 AM <npostavs <at> users.sourceforge.net> wrote:

> Kaushal Modi <kaushal.modi <at> gmail.com> writes:
>
> > +
> > +The PROGRAM is executed asynchronously unless `system-type' is
> > +`windows-nt' or `ms-dos', or unless SYNCH is non-nil.
>
> I think the second "unless" is redundant.
>

I prefer repeating 'unless' in this case where the subject is changing from
system-type to SYNCH. It just adds clarity. Though, grammatically what you
suggested is also correct. So for reducing redundancy, I'll go with your
suggestion.


> > 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.
>
> But OPTIONS can't have nil or "" in it (or any "elements", really), so I
> don't think there is any need to talk about that in the docstring.
>

That's correct. I've updated the patch with what you suggested earlier.


From 086a11c99e77825befc466f65f213d4857618f6f 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 | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/lisp/vc/ediff-diff.el b/lisp/vc/ediff-diff.el
index cfa08ef360..ded82c41c9 100644
--- a/lisp/vc/ediff-diff.el
+++ b/lisp/vc/ediff-diff.el
@@ -1134,12 +1134,20 @@ 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 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; nil and \"\" elements
+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 +1159,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


-- 

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

Merged 26378 26385. Request was from Noam Postavsky <npostavs <at> users.sourceforge.net> to control <at> debbugs.gnu.org. (Thu, 06 Apr 2017 17:16:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Thu, 06 Apr 2017 23:38:02 GMT) Full text and rfc822 format available.

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

From: npostavs <at> users.sourceforge.net
To: Kaushal Modi <kaushal.modi <at> gmail.com>
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 19:38:26 -0400
Kaushal Modi <kaushal.modi <at> gmail.com> writes:

>> > +The PROGRAM is executed asynchronously unless `system-type' is
>> > +`windows-nt' or `ms-dos', or unless SYNCH is non-nil.
>>
>> I think the second "unless" is redundant.
>>
>
> I prefer repeating 'unless' in this case where the subject is changing from
> system-type to SYNCH. It just adds clarity. Though, grammatically what you
> suggested is also correct. So for reducing redundancy, I'll go with your
> suggestion.

Yeah, I guess it could go either way.

>
>> > 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.
>>
>> But OPTIONS can't have nil or "" in it (or any "elements", really), so I
>> don't think there is any need to talk about that in the docstring.
>>
>
> That's correct. I've updated the patch with what you suggested earlier.

Thanks, I think this is ready to go in.  Could you resend as an
attachment?  It doesn't seem to apply, possibly because of line wrapping
and/or whitespace corruption.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Fri, 07 Apr 2017 14:10:01 GMT) Full text and rfc822 format available.

Message #37 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: Fri, 07 Apr 2017 14:09:33 +0000
[Message part 1 (text/plain, inline)]
On Thu, Apr 6, 2017 at 7:37 PM <npostavs <at> users.sourceforge.net> wrote:

> Thanks, I think this is ready to go in.  Could you resend as an
> attachment?  It doesn't seem to apply, possibly because of line wrapping
> and/or whitespace corruption.
>

Thanks. The same patch is now attached.
-- 

Kaushal Modi
[Message part 2 (text/html, inline)]
[0001-Check-that-file-argument-is-a-string.patch (text/x-patch, attachment)]

Merged 26378 26385 26394. Request was from Noam Postavsky <npostavs <at> users.sourceforge.net> to control <at> debbugs.gnu.org. (Fri, 07 Apr 2017 20:42:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Fri, 07 Apr 2017 22:30:02 GMT) Full text and rfc822 format available.

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

From: npostavs <at> users.sourceforge.net
To: Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: phst <at> google.com, 26378 <at> debbugs.gnu.org
Subject: Re: bug#26378: 26.0.50; Hitting 'n' during ediff gives Error
Date: Fri, 07 Apr 2017 18:31:09 -0400
tags 26378 fixed
close 26378 
quit

Kaushal Modi <kaushal.modi <at> gmail.com> writes:

> On Thu, Apr 6, 2017 at 7:37 PM <npostavs <at> users.sourceforge.net> wrote:
>
> Thanks. The same patch is now attached.

Thanks, pushed to master [1: 7582497785].

1: 2017-04-07 18:29:28 -0400 75824977851f27146638672bba4d3789f2a32612
  Check that file argument is a string




Added tag(s) fixed. Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Fri, 07 Apr 2017 22:30:03 GMT) Full text and rfc822 format available.

bug closed, send any further explanations to 26378 <at> debbugs.gnu.org and Kaushal Modi <kaushal.modi <at> gmail.com> Request was from npostavs <at> users.sourceforge.net to control <at> debbugs.gnu.org. (Fri, 07 Apr 2017 22:30:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Sat, 08 Apr 2017 15:00:02 GMT) Full text and rfc822 format available.

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

From: Philipp Stephani <p.stephani2 <at> gmail.com>
To: npostavs <at> users.sourceforge.net, Kaushal Modi <kaushal.modi <at> gmail.com>
Cc: phst <at> google.com, 26378 <at> debbugs.gnu.org
Subject: Re: bug#26378: 26.0.50; Hitting 'n' during ediff gives Error
Date: Sat, 08 Apr 2017 14:58:46 +0000
[Message part 1 (text/plain, inline)]
<npostavs <at> users.sourceforge.net> schrieb am Sa., 8. Apr. 2017 um 00:30 Uhr:

> Kaushal Modi <kaushal.modi <at> gmail.com> writes:
>
> > On Thu, Apr 6, 2017 at 7:37 PM <npostavs <at> users.sourceforge.net> wrote:
> >
> > Thanks. The same patch is now attached.
>
> Thanks, pushed to master [1: 7582497785].
>
> 1: 2017-04-07 18:29:28 -0400 75824977851f27146638672bba4d3789f2a32612
>   Check that file argument is a string
>
>
>
>
Sorry for the breakage, and thanks for the fix! I've added a regression
test (5ea696fd24) to make sure we won't get hit by this again.
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26378; Package emacs. (Mon, 10 Apr 2017 19:38:02 GMT) Full text and rfc822 format available.

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

From: Kaushal Modi <kaushal.modi <at> gmail.com>
To: Philipp Stephani <p.stephani2 <at> gmail.com>, npostavs <at> users.sourceforge.net
Cc: 26378 <at> debbugs.gnu.org
Subject: Re: bug#26378: 26.0.50; Hitting 'n' during ediff gives Error
Date: Mon, 10 Apr 2017 19:36:51 +0000
[Message part 1 (text/plain, inline)]
On Sat, Apr 8, 2017 at 10:59 AM Philipp Stephani <p.stephani2 <at> gmail.com>
wrote:

>
> Sorry for the breakage, and thanks for the fix! I've added a regression
> test (5ea696fd24) to make sure we won't get hit by this again.
>

No problem. Thanks for writing the test. I wanted to write one, but I still
haven't written a single test, so that keeps putting me off from test
writing.

I'll use your test as I example as it will make more sense as I know this
bug well and what the test tests :)


-- 

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

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 09 May 2017 11:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 8 years and 39 days ago.

Previous Next


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