rcirc bug

rcirc bug

[Alfred M. Szmidt]

Hey,

rcirc tries to use member* without (require 'cl), which causes
breakage.

Debugger entered--Lisp error: (void-function member*)
  member*(nick nil)
  rcirc-record-activity(#<buffer *irc.freenode.net*> nick)
  rcirc-print(#<process irc.freenode.net> "RCIRC" "ERROR" nil "\":IceD^!n=iced@86.57.155.204 PRIVMSG #emacs :which gnus backend should I use to read mail from the ~/.mail maildir (one file per message, cur/new/tmp subdirs)\" (void-function member*)" t)
  byte-code("Ã\bÄÅÆÇÈ	\n#É&\x06‡" [process text err rcirc-print "RCIRC" "ERROR" nil format "\"%s\" %s" t] 9)
  rcirc-process-server-response(#<process irc.freenode.net> ":IceD^!n=iced@86.57.155.204 PRIVMSG #emacs :which gnus backend should I use to read mail from the ~/.mail maildir (one file per message, cur/new/tmp subdirs)")
  #[(line) "Â\b	\"‡" [process line rcirc-process-server-response] 3](":IceD^!n=iced@86.57.155.204 PRIVMSG #emacs :which gnus backend should I use to read mail from the ~/.mail maildir (one file per message, cur/new/tmp subdirs)")
  mapc(#[(line) "Â\b	\"‡" [process line rcirc-process-server-response] 3] (":IceD^!n=iced@86.57.155.204 PRIVMSG #emacs :which gnus backend should I use to read mail from the ~/.mail maildir (one file per message, cur/new/tmp subdirs)"))
  rcirc-filter(#<process irc.freenode.net> ":IceD^!n=iced@86.57.155.204 PRIVMSG #emacs :which gnus backend should I use to read mail from the ~/.mail maildir (one file per message, cur/new/tmp subdirs)\n")

Re: rcirc bug

[Richard Stallman]

    rcirc tries to use member* without (require 'cl), which causes
    breakage.

Adding (require 'cl) is not an acceptable solution,
since we don't want Lisp packages to load cl at run time.

rcirc needs to avoid calling member*.

Re: rcirc bug

[Ryan Yeske]

"Alfred M. Szmidt" <ams@gnu.org> writes:

> Hey,
>
> rcirc tries to use member* without (require 'cl), which causes
> breakage.

The following patch fixes this:

*** rcirc.el	05 Sep 2006 12:14:49 -0700	1.26
--- rcirc.el	05 Sep 2006 14:22:05 -0700	
***************
*** 1544,1552 ****
  		 (with-current-buffer b
  		   (dolist (type rcirc-activity-types)
  		     (rcirc-add-face 0 (length s)
! 				     (case type
! 				       ('nick 'rcirc-track-nick)
! 				       ('keyword 'rcirc-track-keyword))
  				     s)))
  		 s))
  	     buffers ","))
--- 1544,1552 ----
  		 (with-current-buffer b
  		   (dolist (type rcirc-activity-types)
  		     (rcirc-add-face 0 (length s)
! 				     (cond
! 				      ((eq type 'nick) 'rcirc-track-nick)
! 				      ((eq type 'keyword) 'rcirc-track-keyword))
  				     s)))
  		 s))
  	     buffers ","))
***************
*** 1936,1945 ****
  (defun rcirc-markup-attributes (process sender response channel-buffer)
    (while (re-search-forward "\\([\C-b\C-_\C-v]\\).*?\\(\\1\\|\C-o\\)" nil t)
      (rcirc-add-face (match-beginning 0) (match-end 0)
! 		    (case (char-after (match-beginning 1))
! 		      (?\C-b 'bold)
! 		      (?\C-v 'italic)
! 		      (?\C-_ 'underline)))
      ;; keep the ^O since it could terminate other attributes
      (when (not (eq ?\C-o (char-before (match-end 2))))
        (delete-region (match-beginning 2) (match-end 2)))
--- 1936,1945 ----
  (defun rcirc-markup-attributes (process sender response channel-buffer)
    (while (re-search-forward "\\([\C-b\C-_\C-v]\\).*?\\(\\1\\|\C-o\\)" nil t)
      (rcirc-add-face (match-beginning 0) (match-end 0)
! 		    (let ((ch (char-after (match-beginning 1))))
! 		      (cond ((eq ch ?\C-b) 'bold)
! 			    ((eq ch ?\C-v) 'italic)
! 			    ((eq ch ?\C-_) 'underline))))
      ;; keep the ^O since it could terminate other attributes
      (when (not (eq ?\C-o (char-before (match-end 2))))
        (delete-region (match-beginning 2) (match-end 2)))

Re: rcirc bug

[Alfred M. Szmidt]

   > rcirc tries to use member* without (require 'cl), which causes
   > breakage.

   The following patch fixes this:

I don't think it will fix the problem in full; pushnew is also a 'cl
macro.  Maybe just changing pushnew to add-to-list would work?  I
don't know if there are more 'cl-isms in rcirc...

(defun rcirc-record-activity (buffer &optional type)
  "Record BUFFER activity with TYPE."
  (with-current-buffer buffer
    (when (not (get-buffer-window (current-buffer) t))
      (setq rcirc-activity
	    (sort (add-to-list 'rcirc-activity (current-buffer))
		  (lambda (b1 b2)
		    (let ((t1 (with-current-buffer b1 rcirc-last-post-time))
			  (t2 (with-current-buffer b2 rcirc-last-post-time)))
		      (time-less-p t2 t1)))))
      (pushnew type rcirc-activity-types)
      (rcirc-update-activity-string)))
  (run-hook-with-args 'rcirc-activity-hooks buffer))

Re: rcirc bug

[Ryan Yeske]

   From: "Alfred M. Szmidt" <ams@gnu.org>
   CC: emacs-devel@gnu.org
   Reply-to: ams@gnu.org
   Date: Tue,  5 Sep 2006 23:37:20 +0200 (CEST)

      > rcirc tries to use member* without (require 'cl), which causes
      > breakage.

      The following patch fixes this:

   I don't think it will fix the problem in full; pushnew is also a 'cl
   macro.  Maybe just changing pushnew to add-to-list would work?  I
   don't know if there are more 'cl-isms in rcirc...

I thought using cl macros was ok, as the .elc file won't require 'cl
at runtime.

Ryan

Re: rcirc bug

[Magnus Henoch]

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

Ryan Yeske <rcyeske@gmail.com> writes:

> The following patch fixes this:

So does this patch, but it gets to keep the `case'.  `case' uses
`member*' only when there is more than one alternative in a clause.
Using a quoted value in a case clause counts as more than one
alternative, as 'nick is equal to (quote nick), but values in case
clauses are not evaluated.


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

*** orig/lisp/net/rcirc.el
--- mod/lisp/net/rcirc.el
***************
*** 1545,1552 ****
  		   (dolist (type rcirc-activity-types)
  		     (rcirc-add-face 0 (length s)
  				     (case type
! 				       ('nick 'rcirc-track-nick)
! 				       ('keyword 'rcirc-track-keyword))
  				     s)))
  		 s))
  	     buffers ","))
--- 1545,1552 ----
  		   (dolist (type rcirc-activity-types)
  		     (rcirc-add-face 0 (length s)
  				     (case type
! 				       (nick 'rcirc-track-nick)
! 				       (keyword 'rcirc-track-keyword))
  				     s)))
  		 s))
  	     buffers ","))

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

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: rcirc bug

[Stefan Monnier]

> "Alfred M. Szmidt" <ams@gnu.org> writes:
>> Hey,
>> 
>> rcirc tries to use member* without (require 'cl), which causes
>> breakage.

> The following patch fixes this:

Odd.  `case' is perfectly OK since it only needs
(eval-when-compile (require 'cl)), which rcirc.el already has.


        Stefan


> *** rcirc.el	05 Sep 2006 12:14:49 -0700	1.26
> --- rcirc.el	05 Sep 2006 14:22:05 -0700	
> ***************
> *** 1544,1552 ****
>   		 (with-current-buffer b
>   		   (dolist (type rcirc-activity-types)
>   		     (rcirc-add-face 0 (length s)
> ! 				     (case type
> ! 				       ('nick 'rcirc-track-nick)
> ! 				       ('keyword 'rcirc-track-keyword))
>   				     s)))
>   		 s))
>   	     buffers ","))
> --- 1544,1552 ----
>   		 (with-current-buffer b
>   		   (dolist (type rcirc-activity-types)
>   		     (rcirc-add-face 0 (length s)
> ! 				     (cond
> ! 				      ((eq type 'nick) 'rcirc-track-nick)
> ! 				      ((eq type 'keyword) 'rcirc-track-keyword))
>   				     s)))
>   		 s))
>   	     buffers ","))
> ***************
> *** 1936,1945 ****
>   (defun rcirc-markup-attributes (process sender response channel-buffer)
>     (while (re-search-forward "\\([\C-b\C-_\C-v]\\).*?\\(\\1\\|\C-o\\)" nil t)
>       (rcirc-add-face (match-beginning 0) (match-end 0)
> ! 		    (case (char-after (match-beginning 1))
> ! 		      (?\C-b 'bold)
> ! 		      (?\C-v 'italic)
> ! 		      (?\C-_ 'underline)))
>       ;; keep the ^O since it could terminate other attributes
>       (when (not (eq ?\C-o (char-before (match-end 2))))
>         (delete-region (match-beginning 2) (match-end 2)))
> --- 1936,1945 ----
>   (defun rcirc-markup-attributes (process sender response channel-buffer)
>     (while (re-search-forward "\\([\C-b\C-_\C-v]\\).*?\\(\\1\\|\C-o\\)" nil t)
>       (rcirc-add-face (match-beginning 0) (match-end 0)
> ! 		    (let ((ch (char-after (match-beginning 1))))
> ! 		      (cond ((eq ch ?\C-b) 'bold)
> ! 			    ((eq ch ?\C-v) 'italic)
> ! 			    ((eq ch ?\C-_) 'underline))))
>       ;; keep the ^O since it could terminate other attributes
>       (when (not (eq ?\C-o (char-before (match-end 2))))
>         (delete-region (match-beginning 2) (match-end 2)))


> _______________________________________________
> Emacs-devel mailing list
> Emacs-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: rcirc bug

[Richard Stallman]

Most cl macros are ok to use.
You need to do (eval-when-compile (require 'cl)).
If the resulting compiled file works without loading cl
then it is ok.

Re: rcirc bug

[Ryan Yeske]

   Most cl macros are ok to use.
   You need to do (eval-when-compile (require 'cl)).
   If the resulting compiled file works without loading cl
   then it is ok.

Then I think everything is OK.

Re: rcirc bug

[Chong Yidong]

Magnus Henoch <mange@freemail.hu> writes:

> Ryan Yeske <rcyeske@gmail.com> writes:
>
>> The following patch fixes this:
>
> So does this patch, but it gets to keep the `case'.  `case' uses
> `member*' only when there is more than one alternative in a clause.
> Using a quoted value in a case clause counts as more than one
> alternative, as 'nick is equal to (quote nick), but values in case
> clauses are not evaluated.

I believe this is right.  Since no one seems to have any objections,
I've checked it into CVS.

> *** orig/lisp/net/rcirc.el
> --- mod/lisp/net/rcirc.el
> ***************
> *** 1545,1552 ****
>   		   (dolist (type rcirc-activity-types)
>   		     (rcirc-add-face 0 (length s)
>   				     (case type
> ! 				       ('nick 'rcirc-track-nick)
> ! 				       ('keyword 'rcirc-track-keyword))
>   				     s)))
>   		 s))
>   	     buffers ","))
> --- 1545,1552 ----
>   		   (dolist (type rcirc-activity-types)
>   		     (rcirc-add-face 0 (length s)
>   				     (case type
> ! 				       (nick 'rcirc-track-nick)
> ! 				       (keyword 'rcirc-track-keyword))
>   				     s)))
>   		 s))
>   	     buffers ","))

Re: rcirc bug

[Stefan Monnier]

>   		   (dolist (type rcirc-activity-types)
>   		     (rcirc-add-face 0 (length s)
>   				     (case type
> ! 				       ('nick 'rcirc-track-nick)
> ! 				       ('keyword 'rcirc-track-keyword))
>   				     s)))
>   		 s))
>   	     buffers ","))

Indeed, this was wrong.  It was equivalent to:

		   (dolist (type rcirc-activity-types)
		     (rcirc-add-face 0 (length s)
				     (cond
                                      ((or (eq type 'nick)
                                           (eq type 'quote))
                                       'rcirc-track-nick)
                                      ((or (eq type 'keyword)
                                           (eq type 'quote))
				       'rcirc-track-keyword))
				     s)))
		 s))
	     buffers ","))


-- Stefan

Re: rcirc bug

[Ryan Yeske]

   From: "Alfred M. Szmidt" <ams@gnu.org>
   CC: emacs-devel@gnu.org
   Reply-to: ams@gnu.org
   Date: Tue,  5 Sep 2006 23:37:20 +0200 (CEST)

      > rcirc tries to use member* without (require 'cl), which causes
      > breakage.

      The following patch fixes this:

   I don't think it will fix the problem in full; pushnew is also a 'cl
   macro.  Maybe just changing pushnew to add-to-list would work?  I
   don't know if there are more 'cl-isms in rcirc...

That is true, case seems to be fine, pushnew is the culprit, as it
the function cl-adjoin is called, which includes member*.

I believe this is the correct fix:

*** rcirc.el	05 Sep 2006 12:14:49 -0700	1.26
--- rcirc.el	07 Sep 2006 15:56:32 -0700	
***************
*** 1495,1501 ****
  		    (let ((t1 (with-current-buffer b1 rcirc-last-post-time))
  			  (t2 (with-current-buffer b2 rcirc-last-post-time)))
  		      (time-less-p t2 t1)))))
!       (pushnew type rcirc-activity-types)
        (rcirc-update-activity-string)))
    (run-hook-with-args 'rcirc-activity-hooks buffer))
  
--- 1501,1507 ----
  		    (let ((t1 (with-current-buffer b1 rcirc-last-post-time))
  			  (t2 (with-current-buffer b2 rcirc-last-post-time)))
  		      (time-less-p t2 t1)))))
!       (add-to-list 'rcirc-activity-types type)
        (rcirc-update-activity-string)))
    (run-hook-with-args 'rcirc-activity-hooks buffer))

Re: rcirc bug

[Richard Stallman]

I think it would be better to change pushnew to expand
into code that does not call any cl functions.  Then programs
that load cl at compile time could use pushnew without problems.

Would someone like to try that?

RE: rcirc bug

[Drew Adams]

    I think it would be better to change pushnew to expand
    into code that does not call any cl functions.  Then programs
    that load cl at compile time could use pushnew without problems.
    
That would be great.

Re: rcirc bug

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> I think it would be better to change pushnew to expand
> into code that does not call any cl functions.  Then programs
> that load cl at compile time could use pushnew without problems.

Is there really a problem?  Can someone tell me how to reproduce this
with a simple test case?  I tried made a file with the following
contents:

  (eval-when-compile (require 'cl))
  (defvar foo nil)
  (defun foobar () (interactive) (pushnew 1 foo))
 
Upon byte compiling and loading the byte-compiled file, `M-x foobar'
seemed to work fine.

Didn't the rcirc bug turn out to come from another source (incorrect
use of `case')?

Re: rcirc bug

[Stuart D. Herring]

> Is there really a problem?  Can someone tell me how to reproduce this
> with a simple test case?  I tried made a file with the following
> contents:
>
>   (eval-when-compile (require 'cl))
>   (defvar foo nil)
>   (defun foobar () (interactive) (pushnew 1 foo))
>
> Upon byte compiling and loading the byte-compiled file, `M-x foobar'
> seemed to work fine.

I don't know about `pushnew' in particular, but in order to test this, you
need to byte compile, then load the file into a new Emacs (preferably with
-Q, of course) and test its operation there.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: rcirc bug

[Richard Stallman]

    > I think it would be better to change pushnew to expand
    > into code that does not call any cl functions.  Then programs
    > that load cl at compile time could use pushnew without problems.

    Is there really a problem?

Yes: all uses of pushnew expanded into calls to `adjoin'.
I changed the simple case yesterday so that it works
without calling `adjoin'.