23.0.50; ielm doesn't handle comments

23.0.50; ielm doesn't handle comments

[Adam Winiecki]

--text follows this line--

while using ielm in emacs 23.0.50 and emacs 22.1, any code run in ielm
with a comment will cause ielm to fail:

ELISP> (message "hi") ;;ouch
*** IELM error ***  More than one sexp in input

ELISP> (setq foo 1) ;;ouch
*** IELM error ***  More than one sexp in input


-atw

Re: 23.0.50; ielm doesn't handle comments

[Lawrence Mitchell]

Adam Winiecki wrote:
> --text follows this line--

> while using ielm in emacs 23.0.50 and emacs 22.1, any code run in ielm
> with a comment will cause ielm to fail:

> ELISP> (message "hi") ;;ouch
> *** IELM error ***  More than one sexp in input

> ELISP> (setq foo 1) ;;ouch
> *** IELM error ***  More than one sexp in input

This didn't work in 21 either.  The problem is that
ielm-eval-input considers a comment on the end of the final input
line to be a second form (it isn't ignore like whitespace is).

The following patch, while perhaps not TRT fixes this problem.



diff --git a/lisp/ielm.el b/lisp/ielm.el
index 764e78b..8a77ba8 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -300,9 +300,9 @@ simply inserts a newline."
 
 ;;; Utility functions
 
-(defun ielm-is-whitespace (string)
+(defun ielm-is-whitespace-or-comment (string)
   "Return non-nil if STRING is all whitespace."
-  (or (string= string "") (string-match "\\`[ \t\n]+\\'" string)))
+  (or (string= string "") (string-match "\\`[ \t\n]+\\(?:;.*\\)*\\'" string)))
 
 ;;; Evaluation
 
@@ -327,7 +327,7 @@ simply inserts a newline."
 	(ielm-output	"")		; result to display
 	(ielm-wbuf ielm-working-buffer)	; current buffer after evaluation
 	(ielm-pmark (ielm-pm)))
-    (if (not (ielm-is-whitespace ielm-string))
+    (if (not (ielm-is-whitespace-or-comment ielm-string))
 	(progn
 	  (condition-case err
 	      (let (rout)
@@ -342,7 +342,8 @@ simply inserts a newline."
 		(setq ielm-result "Working buffer has been killed"
 		      ielm-error-type "IELM Error"
 		      ielm-wbuf (current-buffer))
-	      (if (ielm-is-whitespace (substring ielm-string ielm-pos))
+	      (if (ielm-is-whitespace-or-comment
+                   (substring ielm-string ielm-pos))
 		  ;; To correctly handle the ielm-local variables *,
 		  ;; ** and ***, we need a temporary buffer to be
 		  ;; current at entry to the inner of the next two let


ChangeLog entry:

Lawrence Mitchell <wence@gmx.li>

         * ielm.el (ielm-is-whitespace): Remove.
         (ielm-is-whitespace-or-comment): New function.
         (ielm-eval-input): Use it.

-- 
Lawrence Mitchell <wence@gmx.li>

Re: 23.0.50; ielm doesn't handle comments

[Richard Stallman]

Would people please install this patch if it is correct?

Re: 23.0.50; ielm doesn't handle comments

[Richard Stallman]

    while using ielm in emacs 23.0.50 and emacs 22.1, any code run in ielm
    with a comment will cause ielm to fail:

    ELISP> (message "hi") ;;ouch
    *** IELM error ***  More than one sexp in input

This isn't a terribly important bug, but it would be nice
to fix it.

Re: 23.0.50; ielm doesn't handle comments

[Bastien Guerry]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

Richard Stallman <rms@gnu.org> writes:

>     while using ielm in emacs 23.0.50 and emacs 22.1, any code run in ielm
>     with a comment will cause ielm to fail:
>
>     ELISP> (message "hi") ;;ouch
>     *** IELM error ***  More than one sexp in input
>
> This isn't a terribly important bug, but it would be nice
> to fix it.

This simple patch fixes it.  


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ieml.el.patch --]
[-- Type: text/x-diff, Size: 468 bytes --]

--- ielm.el.~1.58.~	2008-01-08 20:44:50.000000000 +0000
+++ ielm.el	2008-02-14 18:31:34.000000000 +0000
@@ -331,6 +331,9 @@
 	(progn
 	  (condition-case err
 	      (let (rout)
+		;; Make sure there is no comment at the end of the sexp
+		(when (string-match "\\(.*)\\)[ \t]*;+.*$" ielm-string)
+		  (setq ielm-string (replace-match "\\1" t nil ielm-string)))
 		(setq rout (read-from-string ielm-string))
 		(setq ielm-form (car rout))
 		(setq ielm-pos (cdr rout)))

[-- Attachment #3: Type: text/plain, Size: 13 bytes --]


-- 
Bastien

Re: 23.0.50; ielm doesn't handle comments

[Reiner Steib]

On Thu, Feb 14 2008, Bastien Guerry wrote:

> This simple patch fixes it.  

Too simple:

ELISP> "foo ) ; bar"
*** Read error ***  End of file during parsing

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: 23.0.50; ielm doesn't handle comments

[Bastien Guerry]

[-- Attachment #1: Type: text/plain, Size: 249 bytes --]

Reiner Steib <reinersteib+gmane@imap.cc> writes:

> On Thu, Feb 14 2008, Bastien Guerry wrote:
>
>> This simple patch fixes it.  
>
> Too simple:
>
> ELISP> "foo ) ; bar"
> *** Read error ***  End of file during parsing

Right.  Is this one safer?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ieml.el.patch --]
[-- Type: text/x-diff, Size: 856 bytes --]

--- ielm.el.~1.58.~	2008-01-08 20:44:50.000000000 +0000
+++ ielm.el	2008-02-14 19:57:48.000000000 +0000
@@ -304,6 +304,14 @@
   "Return non-nil if STRING is all whitespace."
   (or (string= string "") (string-match "\\`[ \t\n]+\\'" string)))
 
+(defun ielm-trim-comments (string)
+  "Trim comments in STRING."
+  (let ((pos (previous-single-property-change 
+	      (length string) 'face string)))
+    (if (eq (get-text-property pos 'face) 
+	    'font-lock-comment-face)
+	(substring string 0 pos))))
+
 ;;; Evaluation
 
 (defun ielm-eval-input (ielm-string)
@@ -331,6 +339,8 @@
 	(progn
 	  (condition-case err
 	      (let (rout)
+		;; Make sure there is no comment at the end of the sexp
+		(setq ielm-string (ielm-trim-comments ielm-string))
 		(setq rout (read-from-string ielm-string))
 		(setq ielm-form (car rout))
 		(setq ielm-pos (cdr rout)))

[-- Attachment #3: Type: text/plain, Size: 13 bytes --]


-- 
Bastien

Re: 23.0.50; ielm doesn't handle comments

[Lawrence Mitchell]

Bastien Guerry wrote:


[...]


>> (defun ielm-trim-comments (string)
>>  "Trim comments in STRING."
>>  (let ((pos (previous-single-property-change
>>	      (length string) 'face string)))
>>    (if (eq (get-text-property pos 'face)
>>	    'font-lock-comment-face)
>>	(substring string 0 pos))))

This breaks if the user doesn't use font-lock.

[...]


-- 
Lawrence Mitchell <wence@gmx.li>

Re: 23.0.50; ielm doesn't handle comments

[Bastien]

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

Lawrence Mitchell <wence@gmx.li> writes:

> Bastien Guerry wrote:
>
>
> [...]
>
>
>>> (defun ielm-trim-comments (string)
>>>  "Trim comments in STRING."
>>>  (let ((pos (previous-single-property-change
>>>	      (length string) 'face string)))
>>>    (if (eq (get-text-property pos 'face)
>>>	    'font-lock-comment-face)
>>>	(substring string 0 pos))))
>
> This breaks if the user doesn't use font-lock.

Right.  I have come up with another solution, not relying on font-lock,
but this one doesn't allow the double-quote character in comments.

Please improve it if you can.  If no objection/suggestion until next
week, I'll apply this one.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ieml.el.patch --]
[-- Type: text/x-diff, Size: 957 bytes --]

--- ielm.el.~1.58.~	2008-01-08 20:44:50.000000000 +0000
+++ ielm.el	2008-02-15 00:35:37.000000000 +0000
@@ -304,6 +304,17 @@
   "Return non-nil if STRING is all whitespace."
   (or (string= string "") (string-match "\\`[ \t\n]+\\'" string)))
 
+(defun ielm-trim-comments (string)
+  "Trim comments out of STRING."
+  ;; FIXME: comments containing a double-quote character will not be
+  ;; recognized as comments.  This is to allow sexp like "&eacute;".
+  (with-temp-buffer
+    (insert string)
+    (buffer-substring
+     (point-min) 
+     (if (looking-back "[; \t]+[^\"]*" nil t)
+	 (match-beginning 0) (point-max)))))
+
 ;;; Evaluation
 
 (defun ielm-eval-input (ielm-string)
@@ -331,6 +342,8 @@
 	(progn
 	  (condition-case err
 	      (let (rout)
+		;; Make sure to get rid of comments
+		(setq ielm-string (ielm-trim-comments ielm-string))
 		(setq rout (read-from-string ielm-string))
 		(setq ielm-form (car rout))
 		(setq ielm-pos (cdr rout)))

[-- Attachment #3: Type: text/plain, Size: 13 bytes --]


-- 
Bastien

Re: 23.0.50; ielm doesn't handle comments

[Lawrence Mitchell]

Bastien wrote:

[...]

> Right.  I have come up with another solution, not relying on font-lock,
> but this one doesn't allow the double-quote character in comments.

> Please improve it if you can.  If no objection/suggestion until next
> week, I'll apply this one.

I think the right way to fix this is to allow ielm to read the
form as it currently does (using read-from-string) and then
discard following input if it contains either merely whitespace
(as is currently the case) or a comment.

The patch I proposed in
<?fnord?y3h6d4qzmsd3.fsf@ID-97657.user.individual.net>
(http://permalink.gmane.org/gmane.emacs.devel/89087)
does this correctly, I think.

Cheers,

Lawrence

[...]

-- 
Lawrence Mitchell <wence@gmx.li>

Re: 23.0.50; ielm doesn't handle comments

[Bastien Guerry]

[-- Attachment #1: Type: text/plain, Size: 971 bytes --]

Lawrence Mitchell <wence@gmx.li> writes:

> Bastien wrote:
>
> [...]
>
>> Right.  I have come up with another solution, not relying on font-lock,
>> but this one doesn't allow the double-quote character in comments.
>
>> Please improve it if you can.  If no objection/suggestion until next
>> week, I'll apply this one.
>
> I think the right way to fix this is to allow ielm to read the
> form as it currently does (using read-from-string) and then
> discard following input if it contains either merely whitespace
> (as is currently the case) or a comment.

Of course, you're right.  
I haven't seen your first patch, sorry. 

I propose this slightly modified patch below: it uses 

  "\\`[ \t\n]*\\(?:;.*\\)*\\'"
             ^
             ^

in `ielm-is-whitespace-or-comment' so that a sexp like

  (message "test");; comment
                  ^ <= no whitespace

doesn't return an error.

I will apply this with the ChangeLog you provided if you agree.  

Thanks,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ieml.el.patch --]
[-- Type: text/x-diff, Size: 1408 bytes --]

Index: ielm.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/ielm.el,v
retrieving revision 1.58
diff -u -r1.58 ielm.el
--- ielm.el	8 Jan 2008 20:44:50 -0000	1.58
+++ ielm.el	15 Feb 2008 14:19:32 -0000
@@ -300,9 +300,10 @@
 
 ;;; Utility functions
 
-(defun ielm-is-whitespace (string)
+(defun ielm-is-whitespace-or-comment (string)
   "Return non-nil if STRING is all whitespace."
-  (or (string= string "") (string-match "\\`[ \t\n]+\\'" string)))
+  (or (string= string "") 
+      (string-match "\\`[ \t\n]*\\(?:;.*\\)*\\'" string)))
 
 ;;; Evaluation
 
@@ -327,7 +328,7 @@
 	(ielm-output	"")		; result to display
 	(ielm-wbuf ielm-working-buffer)	; current buffer after evaluation
 	(ielm-pmark (ielm-pm)))
-    (if (not (ielm-is-whitespace ielm-string))
+    (if (not (ielm-is-whitespace-or-comment ielm-string))
 	(progn
 	  (condition-case err
 	      (let (rout)
@@ -342,7 +343,8 @@
 		(setq ielm-result "Working buffer has been killed"
 		      ielm-error-type "IELM Error"
 		      ielm-wbuf (current-buffer))
-	      (if (ielm-is-whitespace (substring ielm-string ielm-pos))
+	      (if (ielm-is-whitespace-or-comment
+                   (substring ielm-string ielm-pos))
 		  ;; To correctly handle the ielm-local variables *,
 		  ;; ** and ***, we need a temporary buffer to be
 		  ;; current at entry to the inner of the next two let

[-- Attachment #3: Type: text/plain, Size: 13 bytes --]


-- 
Bastien

Re: 23.0.50; ielm doesn't handle comments

[Bastien Guerry]

Bastien Guerry <bzg@altern.org> writes:

>> I think the right way to fix this is to allow ielm to read the
>> form as it currently does (using read-from-string) and then
>> discard following input if it contains either merely whitespace
>> (as is currently the case) or a comment.
>
> I will apply this with the ChangeLog you provided if you agree.  

Applied.

-- 
Bastien

Re: 23.0.50; ielm doesn't handle comments

[Lawrence Mitchell]

Bastien Guerry wrote:
> Bastien Guerry <bzg@altern.org> writes:

>>> I think the right way to fix this is to allow ielm to read the
>>> form as it currently does (using read-from-string) and then
>>> discard following input if it contains either merely whitespace
>>> (as is currently the case) or a comment.

>> I will apply this with the ChangeLog you provided if you agree.

> Applied.

I realise the patch I sent didn't fix the docstring of
ielm-is-whitespace-or-comment correctly.  How about this:

diff --git a/lisp/ielm.el b/lisp/ielm.el
index a925558..5e1921e 100644
--- a/lisp/ielm.el
+++ b/lisp/ielm.el
@@ -301,7 +301,7 @@ simply inserts a newline."
 ;;; Utility functions
 
 (defun ielm-is-whitespace-or-comment (string)
-  "Return non-nil if STRING is all whitespace."
+  "Return non-nil if STRING is all whitespace or a comment."
   (or (string= string "") 
       (string-match "\\`[ \t\n]*\\(?:;.*\\)*\\'" string)))
 

ChangeLog:

Lawrence Mitchell <wence@gmx.li>

         * ielm.el (ielm-is-whitespace-or-comment): Docstring
           fix.

Cheers,

Lawrence
-- 
Lawrence Mitchell <wence@gmx.li>

Re: 23.0.50; ielm doesn't handle comments

[Bastien]

Lawrence Mitchell <wence@gmx.li> writes:

> Bastien Guerry wrote:
>> Bastien Guerry <bzg@altern.org> writes:
>
>>>> I think the right way to fix this is to allow ielm to read the
>>>> form as it currently does (using read-from-string) and then
>>>> discard following input if it contains either merely whitespace
>>>> (as is currently the case) or a comment.
>
>>> I will apply this with the ChangeLog you provided if you agree.
>
>> Applied.
>
> I realise the patch I sent didn't fix the docstring of
> ielm-is-whitespace-or-comment correctly.  How about this:

Applied, thanks.  I also removed the mention (tiny change) since you
signed the FSF paper -- at least I deduce this from the fact that you
are the author of erc-backend.el which comes with GNU Emacs.  Correct 
me if I'm wrong.

-- 
Bastien

Re: 23.0.50; ielm doesn't handle comments

[Lawrence Mitchell]

Bastien wrote:

[...]

> Applied, thanks.  I also removed the mention (tiny change) since you
> signed the FSF paper -- at least I deduce this from the fact that you
> are the author of erc-backend.el which comes with GNU Emacs.  Correct
> me if I'm wrong.

I've signed papers for ERC (before it was integrated into Emacs),
but don't believe that I have signed papers specifically for
Emacs.

Lawrence
-- 
Lawrence Mitchell <wence@gmx.li>

Re: 23.0.50; ielm doesn't handle comments

[Bastien Guerry]

Lawrence Mitchell <wence@gmx.li> writes:

> Bastien wrote:
>
> [...]
>
>> Applied, thanks.  I also removed the mention (tiny change) since you
>> signed the FSF paper -- at least I deduce this from the fact that you
>> are the author of erc-backend.el which comes with GNU Emacs.  Correct
>> me if I'm wrong.
>
> I've signed papers for ERC (before it was integrated into Emacs),
> but don't believe that I have signed papers specifically for
> Emacs.

All right.  I've mentionned (tiny change) back then.

-- 
Bastien

Re: 23.0.50; ielm doesn't handle comments

[Glenn Morris]

Lawrence Mitchell wrote:

> I've signed papers for ERC (before it was integrated into Emacs),
> but don't believe that I have signed papers specifically for Emacs.

Yup, that's right. If you contribute more we will probably need a
general Emacs assignment. Shall I send you the form?