bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#14974: 24.3.50; adaptive-wrap 0.3 calls potentially void function ea


From: Stefan Monnier
Subject: bug#14974: 24.3.50; adaptive-wrap 0.3 calls potentially void function easy-menu-add-item
Date: Tue, 30 Jul 2013 10:14:05 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux)

> I honestly don't see having the feature check as a waste of code, but no
> matter.

KISS.

> I assume, then, that adaptive-wrap-unload-function is also dispensable;

Yes.

> so is the following patch ok to check in?

Fine, with one comment:
  
> ! (define-key-after (lookup-key menu-bar-options-menu [line-wrapping])
> !   [adaptive-wrap]
> !   '(menu-item "Adaptive Wrap" adaptive-wrap-prefix-mode
> !           :enable t
> !           :visible (menu-bar-menu-frame-live-and-visible-p)
> !           :help "Show wrapped long lines with an adjustable prefix"
> !           :button (:toggle . (bound-and-true-p adaptive-wrap-prefix-mode)))
> !   word-wrap)
  
The ":enable t" is redundant, remove it.
As for the ":visible" entry, I'm wondering what it's there for: I see
it's also used for the line-wrap entry, but I don't know what it's used for
there either.

I see that menu-bar-menu-frame-live-and-visible-p was added in 2005 by
Eli (used for :enable rather than :visible, by the way) and it
was moved to :visible later on when Yidong added the word-wrap entry.

Could someone add a comment explaining what it's for?  Usually the
menu's frame can't be invisible if the menu is visible, so I guess it
has to do with either the `ns' build or with the Gtk feature that lets
on `pin' a menu?  But why is it used on these entries and not on others?
Why use it for `:visible' rather than `:enable' or for something
yet entirely different?


        Stefan





reply via email to

[Prev in Thread] Current Thread [Next in Thread]