bug#4429: purecopy calls needed for :help and in menu-bar.el

[Juri Linkov]

>   > >   > >> Does anyone know a reason why one menu item in menu-bar.el
>   > >   > >> (`separator-exit') doesn't use the standard format with 
> `menu-item'
>   > >   > >> like all other menu items use?
>   > >   > >
>   > >   > > Accident?
>   > >   >
>   > >   > Probably.  Fixed.
>   > >
>   > > I think that a better fix would be to purecopy '("--"), there are >60
>   > > instances of this in GC memory right when emacs starts up.
>   > >
>   > > Maybe define a constant menu-bar-standard-separator and use that
>   > > everywhere?
>   >
>   > I think a constant for "--" is a good idea.
>
> Why not '("--"), it's an extra cons?

That's what I meant - the whole separator thing with cons,
provided it will be located in pure memory.

>   > BTW, I just added a quote in 3 calls of `menu-bar-make-mm-toggle'
>   > in menu-bar.el since this is necessary after your recent changes
>   > in `menu-bar-make-mm-toggle'.
>
> Sorry about that.
>
>   > I hope that no code outside Emacs calls `menu-bar-make-mm-toggle'.
>   > Otherwise, it will be broken.
>
> Do you mean the menu-bar-make-mm-toggle is not incompatible with what it
> was before?  Do you see a way to make it compatible?

Yes, it is incompatible.

1.1. In the old definition:

(defmacro menu-bar-make-mm-toggle (fname doc help &optional props)
  `'(menu-item ,doc ,fname
     ,@props
     :help ,help
     :button (:toggle . (and (default-boundp ',fname)
                             (default-value ',fname)))))

1.2. expanded e.g. with:

(macroexpand
 '(menu-bar-make-mm-toggle transient-mark-mode
                           "Active Region Highlighting"
                           "Make text in active region stand out in color"
                           (:enable (not cua-mode))))

1.3. produced the following:

(quote (menu-item "Active Region Highlighting"
                  transient-mark-mode
                  :enable (not cua-mode)
                  :help "Make text in active region stand out in color"
                  :button (:toggle and (default-boundp (quote 
transient-mark-mode))
                                       (default-value (quote 
transient-mark-mode)))))

1.4. evaluated to:

(menu-item "Active Region Highlighting"
           transient-mark-mode
           :enable (not cua-mode)
           :help "Make text in active region stand out in color"
           :button (:toggle and (default-boundp (quote transient-mark-mode))
                                (default-value (quote transient-mark-mode))))

2.1. But the new definition:

(defmacro menu-bar-make-mm-toggle (fname doc help &optional props)
  `(list 'menu-item  (purecopy ,doc) ',fname
         ,@props
         :help (purecopy ,help)
         :button '(:toggle . (and (default-boundp ',fname)
                                  (default-value ',fname)))))

2.2. expanded with:

(macroexpand
 '(menu-bar-make-mm-toggle transient-mark-mode
                           "Active Region Highlighting"
                           "Make text in active region stand out in color"
                           (:enable (not cua-mode))))

2.3. produces:

(list (quote menu-item)
      (purecopy "Active Region Highlighting")
      (quote transient-mark-mode)
      :enable (not cua-mode)
      :help (purecopy "Make text in active region stand out in color")
      :button (quote (:toggle and (default-boundp (quote transient-mark-mode))
                                  (default-value (quote transient-mark-mode)))))
2.4. and evaluates to:

(menu-item "Active Region Highlighting"
           transient-mark-mode
           :enable t
           :help "Make text in active region stand out in color"
           :button (:toggle and (default-boundp (quote transient-mark-mode))
                                (default-value (quote transient-mark-mode))))

Comparing 1.4 and 2.4, you can see that spliced properties in ,@props
are now unquoted.  It is possible to quote elements of ,@props explicitly:

3.1.

(defmacro menu-bar-make-mm-toggle (fname doc help &optional props)
  `(list 'menu-item  (purecopy ,doc) ',fname
         ,@(mapcar (lambda (p) (list 'quote p)) props)
         :help (purecopy ,help)
         :button '(:toggle . (and (default-boundp ',fname)
                                  (default-value ',fname)))))

3.2.

(macroexpand
 '(menu-bar-make-mm-toggle transient-mark-mode
                           "Active Region Highlighting"
                           "Make text in active region stand out in color"
                           (:enable (not cua-mode))))

3.3.

(list (quote menu-item)
      (purecopy "Active Region Highlighting")
      (quote transient-mark-mode)
      (quote :enable) (quote (not cua-mode))
      :help (purecopy "Make text in active region stand out in color")
      :button (quote (:toggle and (default-boundp (quote transient-mark-mode))
                                  (default-value (quote transient-mark-mode)))))

3.4.

(menu-item "Active Region Highlighting"
           transient-mark-mode
           :enable (not cua-mode)
           :help "Make text in active region stand out in color"
           :button (:toggle and (default-boundp (quote transient-mark-mode))
                                (default-value (quote transient-mark-mode))))

Fixed in the repository.

-- 
Juri Linkov
http://www.jurta.org/emacs/