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

Previous Next

Packages: mh-e, emacs;

Reported by: Katsumi Yamaoka <yamaoka <at> jpl.org>

Date: Wed, 6 Jan 2016 01:03:02 UTC

Severity: normal

Found in version 25.0.50

Done: Bill Wohler <wohler <at> newt.com>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Bill Wohler <wohler <at> newt.com>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#22317: closed (25.0.50; mh-e: wrong usage of cl-flet)
Date: Tue, 31 May 2016 00:19:02 +0000
[Message part 1 (text/plain, inline)]
Your message dated Mon, 30 May 2016 17:18:14 -0700
with message-id <40036.1464653894 <at> olgas.newt.com>
and subject line Re: bug#22317: 25.0.50; mh-e: wrong usage of cl-flet
has caused the debbugs.gnu.org bug report #22317,
regarding 25.0.50; mh-e: wrong usage of cl-flet
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
22317: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=22317
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
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 part 3 (message/rfc822, inline)]
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


This bug report was last modified 9 years and 50 days ago.

Previous Next


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