From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Kaushal Modi Newsgroups: gmane.emacs.bugs Subject: bug#26378: 26.0.50; Hitting 'n' during ediff gives Error Date: Thu, 06 Apr 2017 12:23:42 +0000 Message-ID: References: <87zifuv3q5.fsf@users.sourceforge.net> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=001a113fa8141374d4054c7e9587 X-Trace: blaine.gmane.org 1491481531 14854 195.159.176.226 (6 Apr 2017 12:25:31 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 6 Apr 2017 12:25:31 +0000 (UTC) Cc: phst@google.com, 26378@debbugs.gnu.org To: npostavs@users.sourceforge.net Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Apr 06 14:25:26 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cw6TN-0002P2-Sd for geb-bug-gnu-emacs@m.gmane.org; Thu, 06 Apr 2017 14:25:10 +0200 Original-Received: from localhost ([::1]:45487 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cw6TT-0006Ei-P4 for geb-bug-gnu-emacs@m.gmane.org; Thu, 06 Apr 2017 08:25:15 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:35482) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cw6TN-0006Bg-B2 for bug-gnu-emacs@gnu.org; Thu, 06 Apr 2017 08:25:10 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cw6TH-0003oV-0L for bug-gnu-emacs@gnu.org; Thu, 06 Apr 2017 08:25:09 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:36394) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cw6TG-0003nl-Ri for bug-gnu-emacs@gnu.org; Thu, 06 Apr 2017 08:25:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1cw6TG-0003vx-Fm for bug-gnu-emacs@gnu.org; Thu, 06 Apr 2017 08:25:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Kaushal Modi Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 06 Apr 2017 12:25:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 26378 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 26378-submit@debbugs.gnu.org id=B26378.149148144415019 (code B ref 26378); Thu, 06 Apr 2017 12:25:02 +0000 Original-Received: (at 26378) by debbugs.gnu.org; 6 Apr 2017 12:24:04 +0000 Original-Received: from localhost ([127.0.0.1]:34593 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cw6SJ-0003u8-H0 for submit@debbugs.gnu.org; Thu, 06 Apr 2017 08:24:03 -0400 Original-Received: from mail-lf0-f47.google.com ([209.85.215.47]:36038) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cw6SG-0003tW-De for 26378@debbugs.gnu.org; Thu, 06 Apr 2017 08:24:01 -0400 Original-Received: by mail-lf0-f47.google.com with SMTP id x137so24983844lff.3 for <26378@debbugs.gnu.org>; Thu, 06 Apr 2017 05:24:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L5rarzgm+IkySo5P2502dsd7i7Rx9NHdQ1KCWj/P6xA=; b=WjDPuGA0RVlK+JZj8nid49cq2e1YzpbH+kFptC/qJrQVZ5PpAApfgDJkMzvSngdvkx 2zcZJQGhfF+9asvsXWAsVsG6jd4nrdRJHcZ6vrIAjcw/2pcHZs4EWtA+oGsbXX3mrULe 8loBeCLpNHNcVEBBAGTcVcGntH8bgqcEQT7b2ZKw9Gm44a8Cyo/mU8F0d3iBw63HgKbZ Fq1oWDrM+pnQY6NYnCZItS4aKm/SKTFh6EgNBHDbzS15J+ML7sjFPfxF75Vy7cAl6SwP Nt43rEaby8piajOtjpRVy2JUU8XV7gw0yq4S1Xgw79AspRxopXPrqhDIOSd9jMwsIN/d hXrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L5rarzgm+IkySo5P2502dsd7i7Rx9NHdQ1KCWj/P6xA=; b=NfdRaiAXe1tDPaaqZWh4KzD3EwatnKgKTuIsHPYbju2tXsablDNpibusir0A3Q8ERJ QecFxUypjBRglawBw0lhJ8+i5UWCrcin9f1dvHntJkzZM9SmlM9DaQ/yMU0cKqY7FGBx M0VmtfsZAAhyEzTC7l0o1vFYsz7Pucg4PKILbfBZvvZLQbkFU+bQjsDPMLnCeOBEJtOt o9+9YB3AQM/H123+tABVeG78WsTdHnO2qV8UDivUmdsqlB5ApDUroTuOu+QeOY5mZPjd MgBVLKwf8mctNIx4ZptiTt0HyfVZBFWDbHLvY1s9ANKAL5oz2MYLhvM57ePRT8Xi4NTD 9HHg== X-Gm-Message-State: AFeK/H1AuTHSkEEbaBdZ6xsmcFq6LZJPxMd26KtR0Pz1PRXZR7zchqXWVFLPbVfoE9qTBZy5Llbbb1l8hIC7mQ== X-Received: by 10.25.150.74 with SMTP id y71mr10495121lfd.167.1491481434221; Thu, 06 Apr 2017 05:23:54 -0700 (PDT) In-Reply-To: <87zifuv3q5.fsf@users.sourceforge.net> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:131305 Archived-At: --001a113fa8141374d4054c7e9587 Content-Type: text/plain; charset=UTF-8 Hi Noam, Thanks for the comments. How does this look? >From 99e290cec79754d8d92ec6dcf3c58594782a677b Mon Sep 17 00:00:00 2001 From: Kaushal Modi 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 wrote: > Now that we're looking at the whole thing, I have a few more comments. > > Kaushal Modi 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 --001a113fa8141374d4054c7e9587 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Noam,

Thanks for the comments.
=

How does this look?

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

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

d= iff --git a/lisp/vc/ediff-diff.el b/lisp/vc/ediff-diff.el
index c= fa08ef360..3b2a85afdd 100644
--- a/lisp/vc/ediff-diff.el
+++ b/lisp/vc/ediff-diff.el
@@ -1134,12 +1134,22 @@ ediff-setup= -diff-regions3
=C2=A0 =C2=A0 ))
=C2=A0
=C2=A0=
-;; Execute PROGRAM asynchronously, unless OS/2, Windows-*, or D= OS, or unless
-;; SYNCH is non-nil.=C2=A0 BUFFER must be a buffer= object, and must be alive.=C2=A0 The
-;; OPTIONS arg is a list o= f options to pass to PROGRAM. It may be a blank
-;; string.=C2=A0= All elements in FILES must be strings.=C2=A0 We also delete nil from
=
-;; args.
=C2=A0(defun ediff-exec-process (program buffer sy= nch options &rest files)
+ =C2=A0"Execute the diff PROGR= AM.
+
+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&= #39; or `ms-dos', or unless SYNCH is non-nil.
+
+OP= TIONS is a string of space-separated options to pass to PROGRAM.=C2=A0 It
+may be a blank string.
+
+FILES is a list of = filenames to pass to PROGRAM.=C2=A0 This list may
+contain a nil = element too.
+
+nil and \"\" elements in OPTI= ONS and FILES are ignored."
=C2=A0 =C2=A0(let ((data (match-= data))
=C2=A0 ;; If this is a buffer job, we are diffing temporary files
=C2=A0 ;; produced by Emacs with ediff-coding-system-for-write, so
@@= -1151,8 +1161,9 @@ ediff-exec-process
=C2=A0 args)
=C2=A0 =C2=A0 = =C2=A0(setq args (append (split-string options)
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (mapcar = (lambda (file)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (file-name-= unquote
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(or (file-lo= cal-copy file) file)))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (= when (stringp file)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 (file-name-unquote
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0(or (file-local-copy file) file))))
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 files)))
=C2=A0 =C2=A0 =C2=A0(setq args = (delete "" (delq nil args))) ; delete nil and "" from a= rguments
=C2=A0 =C2=A0 =C2=A0;; the --binary option, if present, = should be used only for buffer jobs
--=C2=A0
2.11.0


On Wed, Ap= r 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:

> +=C2=A0 "Execute the diff PROGRAM.
> +
> +The PROGRAM output is sent to BUFFER, which must be a buffer object,<= br class=3D"gmail_msg"> > +and must be alive.

It would flow a bit better to say "which must be a live buffer object&= quot;.

I agree.
=C2=A0
> +
> +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 jus= t
drop it.=C2=A0

Makes sense.=C2=A0
=C2=A0
And I'm not sure what the &= quot;-*" in "Windows-*" means;

<= div>It means Windows-95, Windows NT, Windows 8, Windows 10, etc.
= =C2=A0
I think
we should just say "unless `system-type' is `windows-nt' or `m= s-dos'".

I= am fine with that.
=C2=A0
&g= t; +
> +OPTIONS is a list of options to pass to PROGRAM.=C2=A0 It may be a bl= ank
> +string.

This seems to be wrong, OPTIONS is not a list.=C2=A0 It should say "OP= TIONS
is a string of space-separated options to pass to PROGRAM".

You are right; making that = change.
=C2=A0
> +
> +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 \"\&quo= t; elements
are ignored."

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

Kaushal Modi

--001a113fa8141374d4054c7e9587--