Subject: bug#14974: 24.3.50; adaptive-wrap 0.3 calls potentially void function easy-menu-add-item

From: Stefan Monnier <[email-masked]>
Date: Tue, 30 Jul 2013 10:14:05 -0400

> 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