bug#8781: 24.0.50; unload-feature don't restore default key

bug#8781: 24.0.50; unload-feature don't restore default key

[Andreas Röhler]

emacs -Q:

in scratch-buffer

M-x python-mode

M-x describe-key <backspace>

==>

`python-backspace'

OK

M-x unload-feature python

M-x describe-key <backspace>

sends error:

describe-key: Symbol's function definition is void: python-backspace

;;;;;;;;;;

Cheers,

Andreas

--
https://code.launchpad.net/~a-roehler/python-mode/components-python-mode
https://code.launchpad.net/s-x-emacs-werkstatt/


If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
     `bt full' and `xbacktrace'.
For information about debugging Emacs, please read the file
/home/dresden/emacs-20110528/etc/DEBUG.


In GNU Emacs 24.0.50.1 (i686-suse-linux-gnu, X toolkit, Xaw3d scroll bars)
  of 2011-05-28 on myrkwid
Windowing system distributor `The X.Org Foundation', version 11.0.10903000
configured using `configure  '--enable-maintainer-mode' 'CFLAGS=-g -O2 
-fno-optimize-sibling-calls -v -save-temps''

Important settings:
   value of $LC_ALL: nil
   value of $LC_COLLATE: nil
   value of $LC_CTYPE: nil
   value of $LC_MESSAGES: nil
   value of $LC_MONETARY: nil
   value of $LC_NUMERIC: nil
   value of $LC_TIME: nil
   value of $LANG: de_DE.UTF-8
   value of $XMODIFIERS: @im=local
   locale-coding-system: utf-8-unix
   default enable-multibyte-characters: t

Major mode: Python

Minor modes in effect:
   tooltip-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   transient-mark-mode: t

Recent input:
M-x p y t h o n - m o d e <return> C-h k <backspace>
C-x 1 M-x u n l o a d - f e a t u <tab> <return> p
y t h o n <return> C-h k <backspace> M-x r e p o r
t - f <backspace> r <backspace> e m a c s - b u g
<return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Type C-x 1 to delete the help window.
describe-key: Symbol's function definition is void: python-backspace

Load-path shadows:
None found.

Features:
(shadow sort gnus-util time-date mail-extr message sendmail format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mailabbrev mail-utils
gmm-utils mailheader emacsbug loadhist help-fns help-mode view easymenu
comint regexp-opt ring tooltip ediff-hook vc-hooks lisp-float-type
mwheel x-win x-dnd tool-bar dnd fontset image fringe lisp-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew
greek romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer loaddefs
button faces cus-face files text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote make-network-process dbusbind dynamic-setting
system-font-setting font-render-setting x-toolkit x multi-tty emacs)

bug#8730: bug#8781: 24.0.50; unload-feature don't restore default key

[Juanma Barranquero]

> M-x unload-feature python
>
> M-x describe-key <backspace>
>
> sends error:
>
> describe-key: Symbol's function definition is void: python-backspace

and

> M-x python-mode
> M-x unload-feature RET python-mode
> M-x shell
>
> error in process filter: Symbol's function definition is void: python-pdbtrack-track-stack-file
> <<< Press Return to bury the buffer list >>>

unload-feature should surely be smarted, but for the time being, the
only way to fix these is to add a python-unload-function which tries
to undo the changes that unload-feature isn't smart enough to do by
itself.

Try the following patch. It sets [pj]ython-mode buffers to
(default-value 'major-mode), and inferior-python-mode buffers to the
parent of this mode (comint). It also removes the hook that causes
bug#8730, and undoes a couple other changes.

Removing things from after-load-alist is a bit unclean, so I've opted
instead for adding an autoload cookie for `python-after-info-look';
that will avoid an error if the user does

M-x python-mode <RET>
M-x unload-feature <RET> python <RET>
M-x info-look-symbol <RET> ;; etc.

though of course python-mode will be loaded again. Not a common enough
situation to be worth a cleaner fix, I think.  (Though if I'm mistaken
and there's a clean way to remove from after-load-alist, I'm all
ears.)

   Juanma


=== modified file 'lisp/progmodes/python.el'
--- lisp/progmodes/python.el	2011-05-13 12:44:48 +0000
+++ lisp/progmodes/python.el	2011-06-01 16:09:34 +0000
@@ -1915,4 +1915,5 @@
 (declare-function info-lookup-maybe-add-help "info-look" (&rest arg))

+;;;###autoload
 (defun python-after-info-look ()
   "Set up info-look for Python.
@@ -2896,4 +2897,23 @@
   (setq overlay-arrow-position nil))

+(defun python-unload-function ()
+  "Unload the Python library."
+  (let* ((default-mode (default-value 'major-mode))
+         (inferior-mode (or (get 'inferior-python-mode 'derived-mode-parent)
+                            default-mode)))
+    (dolist (buffer (buffer-list))
+      (set-buffer buffer)
+      (cond ((memq major-mode '(python-mode jython-mode))
+             (funcall default-mode))
+            ((eq major-mode 'inferior-python-mode)
+             (funcall inferior-mode)))))
+  (remove-hook 'comint-output-filter-functions
'python-pdbtrack-track-stack-file)
+  (setq minor-mode-alist (assq-delete-all 'python-pdbtrack-is-tracking-p
+                                          minor-mode-alist))
+  (dolist (error '("^No symbol" "^Can't shift all lines enough"))
+    (setq debug-ignored-errors (delete error debug-ignored-errors)))
+  ;; continue standard unloading
+  nil)
+
 (provide 'python)
 (provide 'python-21)

bug#8730: bug#8781: 24.0.50; unload-feature don't restore default key

[Stefan Monnier]

> Try the following patch. It sets [pj]ython-mode buffers to
> (default-value 'major-mode), and inferior-python-mode buffers to the
> parent of this mode (comint).

I guess this part could be part of the generic unload-feature code.


        Stefan

bug#8730: bug#8781: bug#8730: bug#8781: 24.0.50; unload-feature don't restore default key

[Juanma Barranquero]

On Thu, Jun 2, 2011 at 06:11, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> Try the following patch. It sets [pj]ython-mode buffers to
>> (default-value 'major-mode), and inferior-python-mode buffers to the
>> parent of this mode (comint).
>
> I guess this part could be part of the generic unload-feature code.

How can we reliably determine that a symbol represents a major mode
name? Or are you proposing to go through the buffer list and checking
whether their major modes are defined in the package being unloaded?

    Juanma

bug#8730: bug#8781: bug#8730: bug#8781: 24.0.50; unload-feature don't restore default key

[Stefan Monnier]

>>> Try the following patch. It sets [pj]ython-mode buffers to
>>> (default-value 'major-mode), and inferior-python-mode buffers to the
>>> parent of this mode (comint).
>> I guess this part could be part of the generic unload-feature code.
> How can we reliably determine that a symbol represents a major mode
> name?

We don't have to do it for all cases, but recognizing major modes
defined by define-derived-mode is easy: check the
`derived-mode-parent' property.

> Or are you proposing to go through the buffer list and checking
> whether their major modes are defined in the package being unloaded?

Ah, yes, indeed, that's another solution that would work as well.


        Stefan

bug#8781: bug#8730: bug#8781: bug#8730: bug#8781: 24.0.50; unload-feature don't restore default key

[Juanma Barranquero]

On Thu, Jun 2, 2011 at 14:26, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>> Or are you proposing to go through the buffer list and checking
>> whether their major modes are defined in the package being unloaded?
>
> Ah, yes, indeed, that's another solution that would work as well.

I prefer that, because it is more thorough. There's no point in a
buffer keeping a major mode that is going to be undefined.

But anyway, this is a new feature, sort of. I suggest installing my
fix for emacs-23 and then doing what you propose in the trunk.

    Juanma

bug#8781: bug#8730: bug#8781: bug#8730: bug#8781: 24.0.50; unload-feature don't restore default key

[Stefan Monnier]

>>> Or are you proposing to go through the buffer list and checking
>>> whether their major modes are defined in the package being unloaded?
>> Ah, yes, indeed, that's another solution that would work as well.
> I prefer that, because it is more thorough.  There's no point in a
> buffer keeping a major mode that is going to be undefined.

Agreed.

> But anyway, this is a new feature, sort of.

Yes, it's clearly not for emacs-23.


        Stefan

bug#8781: bug#8730: bug#8781: bug#8730: bug#8781: 24.0.50; unload-feature don't restore default key

[Juanma Barranquero]

I've implemented a variant of the change we discussed.

What I do, while unloading feature F, is to look for all buffers
having a major mode M defined in F.

If found, I follow back the chain of derivation until I find a major
mode M' which is not defined in F.

If found, I use M'; if not, I switch the buffer to  "(or (default
'major-mode) 'fundamental-mode)".

That's better IMHO, because text modes will be in text-mode,
programming modes will be in prog-mode, inferior modes will be in
comint-mode, etc.

There's an additional check, for cases where (default 'major-mode)
does the unexpected. For example, I have it set to a function that
calls `set-auto-mode', so funcalling it usually sets the buffer to the
same mode it already was. In that case, I default to fundamental-mode
again.

WDYT?


=== modified file 'lisp/loadhist.el'
--- lisp/loadhist.el	2011-06-08 07:17:26 +0000
+++ lisp/loadhist.el	2011-06-23 10:06:48 +0000
@@ -186,7 +186,8 @@
 	 restore-autoload
 	 (name (symbol-name feature))
          (unload-hook (intern-soft (concat name "-unload-hook")))
-	 (unload-func (intern-soft (concat name "-unload-function"))))
+	 (unload-func (intern-soft (concat name "-unload-function")))
+	 (default-mode (or (default-value 'major-mode) 'fundamental-mode)))
     ;; If FEATURE-unload-function is defined and returns non-nil,
     ;; don't try to do anything more; otherwise proceed normally.
     (unless (and (fboundp unload-func)
@@ -222,6 +223,24 @@
 		     (not (get (cdr y) 'autoload)))
 	    (setq auto-mode-alist
 		  (rassq-delete-all (cdr y) auto-mode-alist)))))
+
+      ;; Change major mode in all buffers using one defined in the
feature being unloaded.
+      (save-current-buffer
+        (dolist (buffer (buffer-list))
+          (set-buffer buffer)
+          (let ((proposed major-mode)
+                (original major-mode))
+            ;; Look for an antecessor mode not defined in the feature
we're processing
+            (while (and proposed (rassq proposed unload-function-defs-list))
+              (setq proposed (get proposed 'derived-mode-parent)))
+            (unless (eq proposed major-mode)
+              ;; Two cases: either proposed is nil, and we want to
switch to the default
+              ;; major mode, or proposed is not nil and not
major-mode, and so we use it.
+              (funcall (or proposed default-mode))
+              ;; This check protects agains the user having a weird
default major-mode;
+              ;; for example, one that calls set-auto-mode (been
here, done that).
+              (when (eq major-mode original) (funcall 'fundamental-mode))))))
+
       (when (fboundp 'elp-restore-function) ; remove ELP stuff first
 	(dolist (elt unload-function-defs-list)
 	  (when (symbolp elt)

bug#8730: bug#8781: bug#8730: bug#8781: 24.0.50; unload-feature don't restore default key

[Stefan Monnier]

> If found, I follow back the chain of derivation until I find a major
> mode M' which is not defined in F.

Sounds good.

> If found, I use M'; if not, I switch the buffer to  "(or (default
> 'major-mode) 'fundamental-mode)".

I'd just use fundamental-mode which is treated by define-derived-mode as
the implicit parent when there's no parent.

> +
> +      ;; Change major mode in all buffers using one defined in the
> feature being unloaded.
> +      (save-current-buffer
> +        (dolist (buffer (buffer-list))
> +          (set-buffer buffer)
> +          (let ((proposed major-mode)
> +                (original major-mode))
> +            ;; Look for an antecessor mode not defined in the feature
> we're processing
> +            (while (and proposed (rassq proposed unload-function-defs-list))
> +              (setq proposed (get proposed 'derived-mode-parent)))
> +            (unless (eq proposed major-mode)
> +              ;; Two cases: either proposed is nil, and we want to
> switch to the default
> +              ;; major mode, or proposed is not nil and not
> major-mode, and so we use it.
> +              (funcall (or proposed default-mode))
> +              ;; This check protects agains the user having a weird
> default major-mode;
> +              ;; for example, one that calls set-auto-mode (been
> here, done that).
> +              (when (eq major-mode original) (funcall 'fundamental-mode))))))
> +

But please move it into its own function.


        Stefan

bug#8781: bug#8730: bug#8781: bug#8730: bug#8781: 24.0.50; unload-feature don't restore default key

[Juanma Barranquero]

> I'd just use fundamental-mode which is treated by define-derived-mode as
> the implicit parent when there's no parent.

OK, that's even simpler (and more robust).

> But please move it into its own function.

OK.

    Juanma