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

From: Stephen Berman <[email-masked]>
Date: Tue, 30 Jul 2013 10:54:09 +0200

On Mon, 29 Jul 2013 21:08:48 -0400 Stefan Monnier <[email-masked]> wrote:

>> You mean (lambda () (interactive) (adaptive-wrap-prefix-mode 'toggle))?
>
> No, I mean `adaptive-wrap-prefix-mode', which will already do the same.

Got it, finally.  Sorry for the denseness.

>> Then the check box remains in the menu after unloading adaptive-wrap.el,
>
> Big deal.  Don't waste code on such a non-issue that will only affect
> maybe 10 people in the next century, 9 of whom won't even notice it
> because they won't look at the menu.
>
>> but it becomes a noop.  Making visibility depend on whether the
>> feature is present is, AFAICS, a way of avoiding a separate
>> adaptive-wrap-unload-function.  Or am I wrong about this, or do you
>> mean something else?
>
> As I said, unloading a package is largely broken, so some left-over
> entries in a menu are the least of your worries in this respect.

I honestly don't see having the feature check as a waste of code, but no
matter.  I assume, then, that adaptive-wrap-unload-function is also
dispensable; so is the following patch ok to check in?

Steve Berman

=== modified file 'packages/adaptive-wrap/adaptive-wrap.el'
*** packages/adaptive-wrap/adaptive-wrap.el	2013-07-29 11:56:03 +0000
--- packages/adaptive-wrap/adaptive-wrap.el	2013-07-30 08:15:54 +0000
***************
*** 4,10 ****
  
  ;; Author: Stephen Berman <[email-masked]>
  ;;         Stefan Monnier <[email-masked]>
! ;; Version: 0.4
  
  ;; This program is free software; you can redistribute it and/or modify
  ;; it under the terms of the GNU General Public License as published by
--- 4,10 ----
  
  ;; Author: Stephen Berman <[email-masked]>
  ;;         Stefan Monnier <[email-masked]>
! ;; Version: 0.5
  
  ;; This program is free software; you can redistribute it and/or modify
  ;; it under the terms of the GNU General Public License as published by
***************
*** 103,127 ****
          (widen)
          (remove-text-properties (point-min) (point-max) '(wrap-prefix nil))))))
  
! ;;;###autoload
! (easy-menu-add-item menu-bar-options-menu
!                     '("Line Wrapping in This Buffer")
!                     ["Adaptive Wrap"
!                      (lambda ()
!                        (interactive)
! 		       (if adaptive-wrap-prefix-mode
! 			   (adaptive-wrap-prefix-mode -1)
! 			 (adaptive-wrap-prefix-mode 1)))
!                      :visible (menu-bar-menu-frame-live-and-visible-p)
!                      :help "Show wrapped long lines with an adjustable prefix"
!                      :style toggle
!                      :selected adaptive-wrap-prefix-mode])
! 
! (defun adaptive-wrap-unload-function ()
!   "Cleanup adaptive-wrap package."
!   (easy-menu-remove-item menu-bar-options-menu
!                          '("Line Wrapping in This Buffer")
!                          "Adaptive Wrap"))
  
  (provide 'adaptive-wrap)
  ;;; adaptive-wrap.el ends here
--- 103,116 ----
          (widen)
          (remove-text-properties (point-min) (point-max) '(wrap-prefix nil))))))
  
! (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)
  
  (provide 'adaptive-wrap)
  ;;; adaptive-wrap.el ends here