GNU bug report logs - #22317
25.0.50; mh-e: wrong usage of cl-flet

Previous Next

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

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

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

From: Katsumi Yamaoka <yamaoka <at> jpl.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.50; mh-e: wrong usage of cl-flet
Date: Wed, 06 Jan 2016 10:01:23 +0900

Hi,

mh-e uses Gnus functions to render MIME messages and uses the
mh-cl-flet macro to modify some of them.  Currently mh-e always
loads cl (see mh-acros.el), so both cl-flet and flet are available
and mh-cl-flet will become cl-flet:
,----
| ;; Emacs 24 renamed flet to cl-flet.
| (defalias 'mh-cl-flet
|   (if (fboundp 'cl-flet)
|       'cl-flet
|     'flet))
`----
However, cl-flet is quite unlike flet, IIUC.  For instance, if
cl-flet is used, the mh-cl-flet code in mh-display-emphasis
,----
| ;; (defun mh-display-emphasis ()
| ;;   "Display graphical emphasis."
| ;;   (when (and mh-graphical-emphasis-flag (mh-small-show-buffer-p))
|        (mh-cl-flet
|         ((article-goto-body ()))      ; shadow this function to do nothing
|         (save-excursion
|           (goto-char (point-min))
|           (article-emphasize)))
| ;;     ))
`----
will be expanded to
,----
| (progn
|   (save-excursion
|     (goto-char (point-min))
|     (article-emphasize)))
`----
whereas if flet is used, it will be expanded to:
,----
| (let* ((vnew (cl-function (lambda nil
|                           (cl-block article-goto-body))))
|        (old (symbol-function 'article-goto-body)))
|   (unwind-protect
|       (progn
|       (fset 'article-goto-body vnew)
|       (save-excursion
|         (goto-char (point-min))
|         (article-emphasize)))
|     (fset 'article-goto-body old)))
`----
Note that the former doesn't achieve the original target, i.e.,
article-goto-body is not modified while running article-emphasize.

I don't know how it damages the behavior of mh-e, but I think it
should be fixed anyway.  If mh-e keeps loading cl as ever,
mh-cl-flet can be:

(defalias 'mh-cl-flet 'flet)

Otherwise use this complete Emacs-Lisp style flet emulation macro
(a copy of gmm-flet that exists in only the Gnus git master):

--8<---------------cut here---------------start------------->8---
(defmacro mh-cl-flet (bindings &rest body)
  "Make temporary overriding function definitions.
This is an analogue of a dynamically scoped `let' that operates on
the function cell of FUNCs rather than their value cell.

\(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
  (require 'cl)
  (if (fboundp 'cl-letf)
      `(cl-letf ,(mapcar (lambda (binding)
			   `((symbol-function ',(car binding))
			     (lambda ,@(cdr binding))))
			 bindings)
	 ,@body)
    `(flet ,bindings ,@body)))
(put 'mh-cl-flet 'lisp-indent-function 1)
(put 'mh-cl-flet 'edebug-form-spec
     '((&rest (sexp sexp &rest form)) &rest form))
--8<---------------cut here---------------end--------------->8---

I'm not the right person to install it since I'm not a mh-e user,
sorry.

Regards,

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

From: Glenn Morris <rgm <at> gnu.org>
To: 22317 <at> debbugs.gnu.org
Cc: Katsumi Yamaoka <yamaoka <at> jpl.org>
Subject: Re: bug#22317: 25.0.50; mh-e: wrong usage of cl-flet
Date: Thu, 14 Jan 2016 12:46:20 -0500

mh-e-devel, FYI:

http://debbugs.gnu.org/22317

Katsumi Yamaoka wrote:

> Hi,
>
> mh-e uses Gnus functions to render MIME messages and uses the
> mh-cl-flet macro to modify some of them.  Currently mh-e always
> loads cl (see mh-acros.el), so both cl-flet and flet are available
> and mh-cl-flet will become cl-flet:
> ,----
> | ;; Emacs 24 renamed flet to cl-flet.
> | (defalias 'mh-cl-flet
> |   (if (fboundp 'cl-flet)
> |       'cl-flet
> |     'flet))
> `----
> However, cl-flet is quite unlike flet, IIUC.  For instance, if
> cl-flet is used, the mh-cl-flet code in mh-display-emphasis
> ,----
> | ;; (defun mh-display-emphasis ()
> | ;;   "Display graphical emphasis."
> | ;;   (when (and mh-graphical-emphasis-flag (mh-small-show-buffer-p))
> |        (mh-cl-flet
> |         ((article-goto-body ()))      ; shadow this function to do nothing
> |         (save-excursion
> |           (goto-char (point-min))
> |           (article-emphasize)))
> | ;;     ))
> `----
> will be expanded to
> ,----
> | (progn
> |   (save-excursion
> |     (goto-char (point-min))
> |     (article-emphasize)))
> `----
> whereas if flet is used, it will be expanded to:
> ,----
> | (let* ((vnew (cl-function (lambda nil
> |                           (cl-block article-goto-body))))
> |        (old (symbol-function 'article-goto-body)))
> |   (unwind-protect
> |       (progn
> |       (fset 'article-goto-body vnew)
> |       (save-excursion
> |         (goto-char (point-min))
> |         (article-emphasize)))
> |     (fset 'article-goto-body old)))
> `----
> Note that the former doesn't achieve the original target, i.e.,
> article-goto-body is not modified while running article-emphasize.
>
> I don't know how it damages the behavior of mh-e, but I think it
> should be fixed anyway.  If mh-e keeps loading cl as ever,
> mh-cl-flet can be:
>
> (defalias 'mh-cl-flet 'flet)
>
> Otherwise use this complete Emacs-Lisp style flet emulation macro
> (a copy of gmm-flet that exists in only the Gnus git master):
>
> --8<---------------cut here---------------start------------->8---
> (defmacro mh-cl-flet (bindings &rest body)
>   "Make temporary overriding function definitions.
> This is an analogue of a dynamically scoped `let' that operates on
> the function cell of FUNCs rather than their value cell.
>
> \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
>   (require 'cl)
>   (if (fboundp 'cl-letf)
>       `(cl-letf ,(mapcar (lambda (binding)
> 			   `((symbol-function ',(car binding))
> 			     (lambda ,@(cdr binding))))
> 			 bindings)
> 	 ,@body)
>     `(flet ,bindings ,@body)))
> (put 'mh-cl-flet 'lisp-indent-function 1)
> (put 'mh-cl-flet 'edebug-form-spec
>      '((&rest (sexp sexp &rest form)) &rest form))
> --8<---------------cut here---------------end--------------->8---
>
> I'm not the right person to install it since I'm not a mh-e user,
> sorry.
>
> Regards,

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

From: Bill Wohler <wohler <at> newt.com>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Katsumi Yamaoka <yamaoka <at> jpl.org>, 22317 <at> debbugs.gnu.org,
 mh-e-devel <at> lists.sourceforge.net
Subject: Re: bug#22317: 25.0.50; mh-e: wrong usage of cl-flet
Date: Thu, 14 Jan 2016 10:42:15 -0800

Thanks for the very detailed message. I'll implement your suggestions in
the next release of MH-E. I'm hoping it may fix some mysterious issues
we've been seeing since Emacs 24 came out.

Glenn Morris <rgm <at> gnu.org> wrote:

> 
> mh-e-devel, FYI:
> 
> http://debbugs.gnu.org/22317
> 
> Katsumi Yamaoka wrote:
> 
> > Hi,
> >
> > mh-e uses Gnus functions to render MIME messages and uses the
> > mh-cl-flet macro to modify some of them.  Currently mh-e always
> > loads cl (see mh-acros.el), so both cl-flet and flet are available
> > and mh-cl-flet will become cl-flet:
> > ,----
> > | ;; Emacs 24 renamed flet to cl-flet.
> > | (defalias 'mh-cl-flet
> > |   (if (fboundp 'cl-flet)
> > |       'cl-flet
> > |     'flet))
> > `----
> > However, cl-flet is quite unlike flet, IIUC.  For instance, if
> > cl-flet is used, the mh-cl-flet code in mh-display-emphasis
> > ,----
> > | ;; (defun mh-display-emphasis ()
> > | ;;   "Display graphical emphasis."
> > | ;;   (when (and mh-graphical-emphasis-flag (mh-small-show-buffer-p))
> > |        (mh-cl-flet
> > |         ((article-goto-body ()))      ; shadow this function to do nothing
> > |         (save-excursion
> > |           (goto-char (point-min))
> > |           (article-emphasize)))
> > | ;;     ))
> > `----
> > will be expanded to
> > ,----
> > | (progn
> > |   (save-excursion
> > |     (goto-char (point-min))
> > |     (article-emphasize)))
> > `----
> > whereas if flet is used, it will be expanded to:
> > ,----
> > | (let* ((vnew (cl-function (lambda nil
> > |                           (cl-block article-goto-body))))
> > |        (old (symbol-function 'article-goto-body)))
> > |   (unwind-protect
> > |       (progn
> > |       (fset 'article-goto-body vnew)
> > |       (save-excursion
> > |         (goto-char (point-min))
> > |         (article-emphasize)))
> > |     (fset 'article-goto-body old)))
> > `----
> > Note that the former doesn't achieve the original target, i.e.,
> > article-goto-body is not modified while running article-emphasize.
> >
> > I don't know how it damages the behavior of mh-e, but I think it
> > should be fixed anyway.  If mh-e keeps loading cl as ever,
> > mh-cl-flet can be:
> >
> > (defalias 'mh-cl-flet 'flet)
> >
> > Otherwise use this complete Emacs-Lisp style flet emulation macro
> > (a copy of gmm-flet that exists in only the Gnus git master):
> >
> > --8<---------------cut here---------------start------------->8---
> > (defmacro mh-cl-flet (bindings &rest body)
> >   "Make temporary overriding function definitions.
> > This is an analogue of a dynamically scoped `let' that operates on
> > the function cell of FUNCs rather than their value cell.
> >
> > \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
> >   (require 'cl)
> >   (if (fboundp 'cl-letf)
> >       `(cl-letf ,(mapcar (lambda (binding)
> > 			   `((symbol-function ',(car binding))
> > 			     (lambda ,@(cdr binding))))
> > 			 bindings)
> > 	 ,@body)
> >     `(flet ,bindings ,@body)))
> > (put 'mh-cl-flet 'lisp-indent-function 1)
> > (put 'mh-cl-flet 'edebug-form-spec
> >      '((&rest (sexp sexp &rest form)) &rest form))
> > --8<---------------cut here---------------end--------------->8---
> >
> > I'm not the right person to install it since I'm not a mh-e user,
> > sorry.
> >
> > Regards,
> 
> 
> 
> ------------------------------------------------------------------------------
> Site24x7 APM Insight: Get Deep Visibility into Application Performance
> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
> Monitor end-to-end web transactions and take corrective actions now
> Troubleshoot faster and improve end-user experience. Signup Now!
> http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140
> _______________________________________________
> mh-e-devel mailing list
> mh-e-devel <at> lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/mh-e-devel
> 

-- 
Bill Wohler <wohler <at> newt.com> aka <Bill.Wohler <at> nasa.gov>
http://www.newt.com/wohler/
GnuPG ID:610BD9AD

Message #16 received at 22317-done <at> debbugs.gnu.org (full text, mbox):

From: Bill Wohler <wohler <at> newt.com>
To: Glenn Morris <rgm <at> gnu.org>
Cc: Katsumi Yamaoka <yamaoka <at> jpl.org>, 22317-done <at> debbugs.gnu.org,
 mh-e-devel <at> lists.sourceforge.net
Subject: Re: bug#22317: 25.0.50; mh-e: wrong usage of cl-flet
Date: Mon, 30 May 2016 17:18:14 -0700

Fixed and committed to emacs-25 release branch:

commit 0992ec3b0bfaf98edce1d08462e9ec8e11d6b6e6
Author: Bill Wohler <wohler <at> newt.com>
Date:   Mon May 30 16:49:37 2016 -0700

    Correct cl-flet usage (Bug#22317)
    
    * mh-compat.el: Rename mh-cl-flet to mh-flet and convert alias to
    macro using patch from Katsumi Yamaoka <yamaoka <at> jpl.org>.
    * mh-thread.el (mh-thread-set-tables):
    * mh-show.el (mh-gnus-article-highlight-citation):
    * mh-mime.el (mh-display-with-external-viewer):
    (mh-mime-display, mh-press-button, mh-push-button):
    (mh-display-emphasis): Call mh-flet instead of mh-cl-flet.

Glenn Morris <rgm <at> gnu.org> wrote:

> mh-e-devel, FYI:
> 
> http://debbugs.gnu.org/22317
> 
> Katsumi Yamaoka wrote:
> 
> > Hi,
> >
> > mh-e uses Gnus functions to render MIME messages and uses the
> > mh-cl-flet macro to modify some of them.  Currently mh-e always
> > loads cl (see mh-acros.el), so both cl-flet and flet are available
> > and mh-cl-flet will become cl-flet:
> > ,----
> > | ;; Emacs 24 renamed flet to cl-flet.
> > | (defalias 'mh-cl-flet
> > |   (if (fboundp 'cl-flet)
> > |       'cl-flet
> > |     'flet))
> > `----
> > However, cl-flet is quite unlike flet, IIUC.  For instance, if
> > cl-flet is used, the mh-cl-flet code in mh-display-emphasis
> > ,----
> > | ;; (defun mh-display-emphasis ()
> > | ;;   "Display graphical emphasis."
> > | ;;   (when (and mh-graphical-emphasis-flag (mh-small-show-buffer-p))
> > |        (mh-cl-flet
> > |         ((article-goto-body ()))      ; shadow this function to do nothing
> > |         (save-excursion
> > |           (goto-char (point-min))
> > |           (article-emphasize)))
> > | ;;     ))
> > `----
> > will be expanded to
> > ,----
> > | (progn
> > |   (save-excursion
> > |     (goto-char (point-min))
> > |     (article-emphasize)))
> > `----
> > whereas if flet is used, it will be expanded to:
> > ,----
> > | (let* ((vnew (cl-function (lambda nil
> > |                           (cl-block article-goto-body))))
> > |        (old (symbol-function 'article-goto-body)))
> > |   (unwind-protect
> > |       (progn
> > |       (fset 'article-goto-body vnew)
> > |       (save-excursion
> > |         (goto-char (point-min))
> > |         (article-emphasize)))
> > |     (fset 'article-goto-body old)))
> > `----
> > Note that the former doesn't achieve the original target, i.e.,
> > article-goto-body is not modified while running article-emphasize.
> >
> > I don't know how it damages the behavior of mh-e, but I think it
> > should be fixed anyway.  If mh-e keeps loading cl as ever,
> > mh-cl-flet can be:
> >
> > (defalias 'mh-cl-flet 'flet)
> >
> > Otherwise use this complete Emacs-Lisp style flet emulation macro
> > (a copy of gmm-flet that exists in only the Gnus git master):
> >
> > --8<---------------cut here---------------start------------->8---
> > (defmacro mh-cl-flet (bindings &rest body)
> >   "Make temporary overriding function definitions.
> > This is an analogue of a dynamically scoped `let' that operates on
> > the function cell of FUNCs rather than their value cell.
> >
> > \(fn ((FUNC ARGLIST BODY...) ...) FORM...)"
> >   (require 'cl)
> >   (if (fboundp 'cl-letf)
> >       `(cl-letf ,(mapcar (lambda (binding)
> > 			   `((symbol-function ',(car binding))
> > 			     (lambda ,@(cdr binding))))
> > 			 bindings)
> > 	 ,@body)
> >     `(flet ,bindings ,@body)))
> > (put 'mh-cl-flet 'lisp-indent-function 1)
> > (put 'mh-cl-flet 'edebug-form-spec
> >      '((&rest (sexp sexp &rest form)) &rest form))
> > --8<---------------cut here---------------end--------------->8---
> >
> > I'm not the right person to install it since I'm not a mh-e user,
> > sorry.
> >
> > Regards,
> 
> 
> 
> ------------------------------------------------------------------------------
> Site24x7 APM Insight: Get Deep Visibility into Application Performance
> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
> Monitor end-to-end web transactions and take corrective actions now
> Troubleshoot faster and improve end-user experience. Signup Now!
> http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140
> _______________________________________________
> mh-e-devel mailing list
> mh-e-devel <at> lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/mh-e-devel
> 

-- 
Bill Wohler <wohler <at> newt.com> aka <Bill.Wohler <at> nasa.gov>
http://www.newt.com/wohler/
GnuPG ID:610BD9AD

Previous Next

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