* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error @ 2017-04-05 20:57 Kaushal Modi 2017-04-05 21:03 ` Kaushal Modi 0 siblings, 1 reply; 14+ messages in thread From: Kaushal Modi @ 2017-04-05 20:57 UTC (permalink / raw) To: 26378 [-- Attachment #1.1: Type: text/plain, Size: 5072 bytes --] 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 [-- Attachment #1.2: Type: text/html, Size: 6733 bytes --] [-- Attachment #2: pasted1 --] [-- Type: image/png, Size: 87958 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-05 20:57 bug#26378: 26.0.50; Hitting 'n' during ediff gives Error Kaushal Modi @ 2017-04-05 21:03 ` Kaushal Modi 2017-04-05 21:20 ` Kaushal Modi 0 siblings, 1 reply; 14+ messages in thread From: Kaushal Modi @ 2017-04-05 21:03 UTC (permalink / raw) To: 26378, phst [-- Attachment #1: Type: text/plain, Size: 152 bytes --] Reverting this commit fixes this bug. http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=f4b50dad8d5eade04f495c693c0bca46060b25cb -- Kaushal Modi [-- Attachment #2: Type: text/html, Size: 396 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-05 21:03 ` Kaushal Modi @ 2017-04-05 21:20 ` Kaushal Modi 2017-04-05 21:53 ` Noam Postavsky 0 siblings, 1 reply; 14+ messages in thread From: Kaushal Modi @ 2017-04-05 21:20 UTC (permalink / raw) To: 26378, phst [-- Attachment #1: Type: text/plain, Size: 1338 bytes --] This patch fixes the error: From 9de1cad8697781ac9a3729087cd38f1da7374ad4 Mon Sep 17 00:00:00 2001 From: Kaushal Modi <kaushal.modi@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 [-- Attachment #2: Type: text/html, Size: 2285 bytes --] ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-05 21:20 ` Kaushal Modi @ 2017-04-05 21:53 ` Noam Postavsky 2017-04-05 22:36 ` Kaushal Modi 0 siblings, 1 reply; 14+ messages in thread From: Noam Postavsky @ 2017-04-05 21:53 UTC (permalink / raw) To: Kaushal Modi; +Cc: phst, 26378 On Wed, Apr 5, 2017 at 5:20 PM, Kaushal Modi <kaushal.modi@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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-05 21:53 ` Noam Postavsky @ 2017-04-05 22:36 ` Kaushal Modi 2017-04-05 23:23 ` npostavs 0 siblings, 1 reply; 14+ messages in thread From: Kaushal Modi @ 2017-04-05 22:36 UTC (permalink / raw) To: Noam Postavsky; +Cc: phst, 26378 [-- Attachment #1: Type: text/plain, Size: 2801 bytes --] On Wed, Apr 5, 2017 at 5:53 PM Noam Postavsky < npostavs@users.sourceforge.net> wrote: > On Wed, Apr 5, 2017 at 5:20 PM, Kaushal Modi <kaushal.modi@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@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 [-- Attachment #2: Type: text/html, Size: 4555 bytes --] ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-05 22:36 ` Kaushal Modi @ 2017-04-05 23:23 ` npostavs 2017-04-06 12:23 ` Kaushal Modi 0 siblings, 1 reply; 14+ messages in thread From: npostavs @ 2017-04-05 23:23 UTC (permalink / raw) To: Kaushal Modi; +Cc: phst, 26378 Now that we're looking at the whole thing, I have a few more comments. Kaushal Modi <kaushal.modi@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." ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-05 23:23 ` npostavs @ 2017-04-06 12:23 ` Kaushal Modi 2017-04-06 12:45 ` npostavs 0 siblings, 1 reply; 14+ messages in thread From: Kaushal Modi @ 2017-04-06 12:23 UTC (permalink / raw) To: npostavs; +Cc: phst, 26378 [-- Attachment #1: Type: text/plain, Size: 4121 bytes --] Hi Noam, Thanks for the comments. How does this look? From 99e290cec79754d8d92ec6dcf3c58594782a677b Mon Sep 17 00:00:00 2001 From: Kaushal Modi <kaushal.modi@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@users.sourceforge.net> wrote: > Now that we're looking at the whole thing, I have a few more comments. > > Kaushal Modi <kaushal.modi@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 [-- Attachment #2: Type: text/html, Size: 7270 bytes --] ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-06 12:23 ` Kaushal Modi @ 2017-04-06 12:45 ` npostavs 2017-04-06 14:36 ` Kaushal Modi 0 siblings, 1 reply; 14+ messages in thread From: npostavs @ 2017-04-06 12:45 UTC (permalink / raw) To: Kaushal Modi; +Cc: phst, 26378 Kaushal Modi <kaushal.modi@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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-06 12:45 ` npostavs @ 2017-04-06 14:36 ` Kaushal Modi 2017-04-06 23:38 ` npostavs 0 siblings, 1 reply; 14+ messages in thread From: Kaushal Modi @ 2017-04-06 14:36 UTC (permalink / raw) To: npostavs; +Cc: phst, 26378 [-- Attachment #1: Type: text/plain, Size: 3320 bytes --] On Thu, Apr 6, 2017 at 8:44 AM <npostavs@users.sourceforge.net> wrote: > Kaushal Modi <kaushal.modi@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@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 [-- Attachment #2: Type: text/html, Size: 5348 bytes --] ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-06 14:36 ` Kaushal Modi @ 2017-04-06 23:38 ` npostavs 2017-04-07 14:09 ` Kaushal Modi 0 siblings, 1 reply; 14+ messages in thread From: npostavs @ 2017-04-06 23:38 UTC (permalink / raw) To: Kaushal Modi; +Cc: phst, 26378 Kaushal Modi <kaushal.modi@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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-06 23:38 ` npostavs @ 2017-04-07 14:09 ` Kaushal Modi 2017-04-07 22:31 ` npostavs 0 siblings, 1 reply; 14+ messages in thread From: Kaushal Modi @ 2017-04-07 14:09 UTC (permalink / raw) To: npostavs; +Cc: phst, 26378 [-- Attachment #1.1: Type: text/plain, Size: 306 bytes --] On Thu, Apr 6, 2017 at 7:37 PM <npostavs@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 [-- Attachment #1.2: Type: text/html, Size: 755 bytes --] [-- Attachment #2: 0001-Check-that-file-argument-is-a-string.patch --] [-- Type: text/x-patch, Size: 2348 bytes --] From 086a11c99e77825befc466f65f213d4857618f6f Mon Sep 17 00:00:00 2001 From: Kaushal Modi <kaushal.modi@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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-07 14:09 ` Kaushal Modi @ 2017-04-07 22:31 ` npostavs 2017-04-08 14:58 ` Philipp Stephani 0 siblings, 1 reply; 14+ messages in thread From: npostavs @ 2017-04-07 22:31 UTC (permalink / raw) To: Kaushal Modi; +Cc: phst, 26378 tags 26378 fixed close 26378 quit Kaushal Modi <kaushal.modi@gmail.com> writes: > On Thu, Apr 6, 2017 at 7:37 PM <npostavs@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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-07 22:31 ` npostavs @ 2017-04-08 14:58 ` Philipp Stephani 2017-04-10 19:36 ` Kaushal Modi 0 siblings, 1 reply; 14+ messages in thread From: Philipp Stephani @ 2017-04-08 14:58 UTC (permalink / raw) To: npostavs, Kaushal Modi; +Cc: phst, 26378 [-- Attachment #1: Type: text/plain, Size: 554 bytes --] <npostavs@users.sourceforge.net> schrieb am Sa., 8. Apr. 2017 um 00:30 Uhr: > Kaushal Modi <kaushal.modi@gmail.com> writes: > > > On Thu, Apr 6, 2017 at 7:37 PM <npostavs@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. [-- Attachment #2: Type: text/html, Size: 1321 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#26378: 26.0.50; Hitting 'n' during ediff gives Error 2017-04-08 14:58 ` Philipp Stephani @ 2017-04-10 19:36 ` Kaushal Modi 0 siblings, 0 replies; 14+ messages in thread From: Kaushal Modi @ 2017-04-10 19:36 UTC (permalink / raw) To: Philipp Stephani, npostavs; +Cc: 26378 [-- Attachment #1: Type: text/plain, Size: 511 bytes --] On Sat, Apr 8, 2017 at 10:59 AM Philipp Stephani <p.stephani2@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 [-- Attachment #2: Type: text/html, Size: 1216 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-04-10 19:36 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-05 20:57 bug#26378: 26.0.50; Hitting 'n' during ediff gives Error Kaushal Modi 2017-04-05 21:03 ` Kaushal Modi 2017-04-05 21:20 ` Kaushal Modi 2017-04-05 21:53 ` Noam Postavsky 2017-04-05 22:36 ` Kaushal Modi 2017-04-05 23:23 ` npostavs 2017-04-06 12:23 ` Kaushal Modi 2017-04-06 12:45 ` npostavs 2017-04-06 14:36 ` Kaushal Modi 2017-04-06 23:38 ` npostavs 2017-04-07 14:09 ` Kaushal Modi 2017-04-07 22:31 ` npostavs 2017-04-08 14:58 ` Philipp Stephani 2017-04-10 19:36 ` Kaushal Modi
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).