* bug#4429: purecopy calls needed for :help and in menu-bar.el
@ 2009-09-14 18:43 Dan Nicolaescu
2009-09-14 21:59 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Dan Nicolaescu @ 2009-09-14 18:43 UTC (permalink / raw)
To: bug-gnu-emacs
emacs/lisp/bindings.el has purecopy calls for menu-item names, but not
for the corresponding :help. Any reason not to add those purecopy calls?
emacs/lisp/menu-bar.el does not have purecopy calls for any menu-items,
and it contains quite a few of them. Any reason not to add purecopy
calls there?
The patch below adds a purecopy call to emacs/lisp/help.el
Is there anything else that could benefit from purecopy calls?
Here's some changes that I have in my local tree. OK to check these in?
Index: lisp/bindings.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/bindings.el,v
retrieving revision 1.224
diff -u -3 -p -r1.224 bindings.el
--- lisp/bindings.el 11 Sep 2009 06:38:44 -0000 1.224
+++ lisp/bindings.el 13 Sep 2009 21:07:47 -0000
@@ -507,49 +507,49 @@ Switch to the most recently selected buf
;; Global ones can go on the menubar (Options --> Show/Hide).
(define-key mode-line-mode-menu [overwrite-mode]
`(menu-item ,(purecopy "Overwrite (Ovwrt)") overwrite-mode
- :help "Overwrite mode: typed characters replace existing text"
+ :help ,(purecopy "Overwrite mode: typed characters replace existing text")
:button (:toggle . overwrite-mode)))
(define-key mode-line-mode-menu [outline-minor-mode]
`(menu-item ,(purecopy "Outline (Outl)") outline-minor-mode
;; XXX: This needs a good, brief description.
- :help ""
+ :help ,(purecopy "")
:button (:toggle . (bound-and-true-p outline-minor-mode))))
(define-key mode-line-mode-menu [highlight-changes-mode]
`(menu-item ,(purecopy "Highlight changes (Chg)") highlight-changes-mode
- :help "Show changes in the buffer in a distinctive color"
+ :help ,(purecopy "Show changes in the buffer in a distinctive color")
:button (:toggle . (bound-and-true-p highlight-changes-mode))))
(define-key mode-line-mode-menu [hide-ifdef-mode]
`(menu-item ,(purecopy "Hide ifdef (Ifdef)") hide-ifdef-mode
- :help "Show/Hide code within #ifdef constructs"
+ :help ,(purecopy "Show/Hide code within #ifdef constructs")
:button (:toggle . (bound-and-true-p hide-ifdef-mode))))
(define-key mode-line-mode-menu [glasses-mode]
`(menu-item ,(purecopy "Glasses (o^o)") glasses-mode
- :help "Insert virtual separators to make long identifiers easy to read"
+ :help ,(purecopy "Insert virtual separators to make long identifiers easy to read")
:button (:toggle . (bound-and-true-p glasses-mode))))
(define-key mode-line-mode-menu [font-lock-mode]
`(menu-item ,(purecopy "Font Lock") font-lock-mode
- :help "Syntax coloring"
+ :help ,(purecopy "Syntax coloring")
:button (:toggle . font-lock-mode)))
(define-key mode-line-mode-menu [flyspell-mode]
`(menu-item ,(purecopy "Flyspell (Fly)") flyspell-mode
- :help "Spell checking on the fly"
+ :help ,(purecopy "Spell checking on the fly")
:button (:toggle . (bound-and-true-p flyspell-mode))))
(define-key mode-line-mode-menu [auto-revert-tail-mode]
`(menu-item ,(purecopy "Auto revert tail (Tail)") auto-revert-tail-mode
- :help "Revert the tail of the buffer when buffer grows"
+ :help ,(purecopy "Revert the tail of the buffer when buffer grows")
:enable (buffer-file-name)
:button (:toggle . (bound-and-true-p auto-revert-tail-mode))))
(define-key mode-line-mode-menu [auto-revert-mode]
`(menu-item ,(purecopy "Auto revert (ARev)") auto-revert-mode
- :help "Revert the buffer when the file on disk changes"
+ :help ,(purecopy "Revert the buffer when the file on disk changes")
:button (:toggle . (bound-and-true-p auto-revert-mode))))
(define-key mode-line-mode-menu [auto-fill-mode]
`(menu-item ,(purecopy "Auto fill (Fill)") auto-fill-mode
- :help "Automatically insert new lines"
+ :help ,(purecopy "Automatically insert new lines")
:button (:toggle . auto-fill-function)))
(define-key mode-line-mode-menu [abbrev-mode]
`(menu-item ,(purecopy "Abbrev (Abbrev)") abbrev-mode
- :help "Automatically expand abbreviations"
+ :help ,(purecopy "Automatically expand abbreviations")
:button (:toggle . abbrev-mode)))
(defun mode-line-minor-mode-help (event)
Index: lisp/help.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/help.el,v
retrieving revision 1.347
diff -u -3 -p -r1.347 help.el
--- lisp/help.el 19 Aug 2009 18:07:10 -0000 1.347
+++ lisp/help.el 13 Sep 2009 21:07:47 -0000
@@ -202,7 +202,8 @@ specifies what to do when the user exits
(defalias 'help-for-help 'help-for-help-internal)
;; It can't find this, but nobody will look.
(make-help-screen help-for-help-internal
- "Type a help option: [abcCdefFgiIkKlLmnprstvw.] C-[cdefmnoptw] or ?"
+ (purecopy "Type a help option: [abcCdefFgiIkKlLmnprstvw.] C-[cdefmnoptw] or ?")
+ (purecopy
"You have typed %THIS-KEY%, the help character. Type a Help option:
\(Use SPC or DEL to scroll through this text. Type \\<help-map>\\[help-quit] to exit the Help command.)
@@ -247,7 +248,7 @@ C-n News of recent Emacs changes
C-o Emacs ordering and distribution information.
C-p Info about known Emacs problems.
C-t Emacs TODO list.
-C-w Information on absence of warranty for GNU Emacs."
+C-w Information on absence of warranty for GNU Emacs.")
help-map)
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-09-14 18:43 bug#4429: purecopy calls needed for :help and in menu-bar.el Dan Nicolaescu
@ 2009-09-14 21:59 ` Stefan Monnier
2009-09-16 10:50 ` Dan Nicolaescu
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2009-09-14 21:59 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: bug-gnu-emacs, 4429
> Here's some changes that I have in my local tree. OK to check these in?
Looks OK to me.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-09-14 21:59 ` Stefan Monnier
@ 2009-09-16 10:50 ` Dan Nicolaescu
2009-09-16 13:20 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Dan Nicolaescu @ 2009-09-16 10:50 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 4429
Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
> > Here's some changes that I have in my local tree. OK to check these in?
>
> Looks OK to me.
How about changing menu-bar-make-mm-toggle and menu-bar-make-toggle to
purecopy their string arguments: (DOC FNAME) and (DOC MESSAGE HELP), respectively?
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-09-16 10:50 ` Dan Nicolaescu
@ 2009-09-16 13:20 ` Stefan Monnier
2009-10-14 20:32 ` Juri Linkov
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2009-09-16 13:20 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: 4429
>> > Here's some changes that I have in my local tree. OK to check these in?
>> Looks OK to me.
> How about changing menu-bar-make-mm-toggle and menu-bar-make-toggle to
> purecopy their string arguments: (DOC FNAME) and (DOC MESSAGE
> HELP), respectively?
If you've checked that it's correct, it sounds good to me,
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-09-16 13:20 ` Stefan Monnier
@ 2009-10-14 20:32 ` Juri Linkov
2009-10-15 3:18 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2009-10-14 20:32 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Dan Nicolaescu, 4429
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?
With the following change it seems to work right, including
"F10 f" on non-toolkit builds and -nw.
Index: lisp/menu-bar.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/menu-bar.el,v
retrieving revision 1.361
diff -u -r1.361 menu-bar.el
--- lisp/menu-bar.el 12 Oct 2009 00:52:26 -0000 1.361
+++ lisp/menu-bar.el 14 Oct 2009 20:31:41 -0000
@@ -68,7 +68,7 @@
:help ,(purecopy "Save unsaved buffers, then exit")))
(define-key menu-bar-file-menu [separator-exit]
- (purecopy '("--")))
+ `(menu-item ,(purecopy "--")))
;; Don't use delete-frame as event name because that is a special
;; event.
--
Juri Linkov
http://www.jurta.org/emacs/
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-10-14 20:32 ` Juri Linkov
@ 2009-10-15 3:18 ` Stefan Monnier
2009-10-15 22:30 ` Juri Linkov
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2009-10-15 3:18 UTC (permalink / raw)
To: Juri Linkov; +Cc: Dan Nicolaescu, 4429
> 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?
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-10-15 3:18 ` Stefan Monnier
@ 2009-10-15 22:30 ` Juri Linkov
2009-11-03 6:45 ` Dan Nicolaescu
0 siblings, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2009-10-15 22:30 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Dan Nicolaescu, 4429
>> 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.
--
Juri Linkov
http://www.jurta.org/emacs/
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-10-15 22:30 ` Juri Linkov
@ 2009-11-03 6:45 ` Dan Nicolaescu
2009-11-03 7:29 ` Juri Linkov
0 siblings, 1 reply; 11+ messages in thread
From: Dan Nicolaescu @ 2009-11-03 6:45 UTC (permalink / raw)
To: Juri Linkov; +Cc: 4429
Juri Linkov <juri@jurta.org> writes:
> >> 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?
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-11-03 6:45 ` Dan Nicolaescu
@ 2009-11-03 7:29 ` Juri Linkov
2009-11-04 1:50 ` Dan Nicolaescu
0 siblings, 1 reply; 11+ messages in thread
From: Juri Linkov @ 2009-11-03 7:29 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: 4429
> > >> 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.
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'.
I hope that no code outside Emacs calls `menu-bar-make-mm-toggle'.
Otherwise, it will be broken.
--
Juri Linkov
http://www.jurta.org/emacs/
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-11-03 7:29 ` Juri Linkov
@ 2009-11-04 1:50 ` Dan Nicolaescu
2009-11-04 2:55 ` Juri Linkov
0 siblings, 1 reply; 11+ messages in thread
From: Dan Nicolaescu @ 2009-11-04 1:50 UTC (permalink / raw)
To: Juri Linkov; +Cc: 4429
Juri Linkov <juri@jurta.org> writes:
> > > >> 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?
> 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?
^ permalink raw reply [flat|nested] 11+ messages in thread
* bug#4429: purecopy calls needed for :help and in menu-bar.el
2009-11-04 1:50 ` Dan Nicolaescu
@ 2009-11-04 2:55 ` Juri Linkov
0 siblings, 0 replies; 11+ messages in thread
From: Juri Linkov @ 2009-11-04 2:55 UTC (permalink / raw)
To: Dan Nicolaescu; +Cc: 4429
> > > > >> 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/
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2009-11-04 2:55 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-14 18:43 bug#4429: purecopy calls needed for :help and in menu-bar.el Dan Nicolaescu
2009-09-14 21:59 ` Stefan Monnier
2009-09-16 10:50 ` Dan Nicolaescu
2009-09-16 13:20 ` Stefan Monnier
2009-10-14 20:32 ` Juri Linkov
2009-10-15 3:18 ` Stefan Monnier
2009-10-15 22:30 ` Juri Linkov
2009-11-03 6:45 ` Dan Nicolaescu
2009-11-03 7:29 ` Juri Linkov
2009-11-04 1:50 ` Dan Nicolaescu
2009-11-04 2:55 ` Juri Linkov
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).