Re: Refreshing Info nodes

[Stefan Monnier <monnier@iro.umontreal.ca>]

> Now there was some problem with buffer local variable
> `revert-buffer-function'. (in doc-view-mode) I corrected it I believe,
> and this is the final version of patch.  Could anybody review that once
> more, please?

Some nitpicks:

> +  ;; This is not interactive because you shouldn't be turning this
> +  ;; mode on and off.  You can corrupt things that way.
> +  ;;
> +  ;; Now it's derived mode, so therefore it is interactive.

These two pieces are contradictory, please clarify.  BTW, we should
change archive-mode to use buffer-swap-text like we did for tar-mode,
which should address this risk of corruption.

> -(defun bookmark-bmenu-mode ()
> +(define-derived-mode bookmark-bmenu-mode special-mode "Bookmark Menu"
>    "Major mode for editing a list of bookmarks.
>  Each line describes one of the bookmarks in Emacs.
>  Letters do not insert themselves; instead, they are commands.
> @@ -1645,9 +1642,6 @@
>    (kill-all-local-variables)
>    (use-local-map bookmark-bmenu-mode-map)

Both kill-all-local-variables and use-local-map can be removed (actually
kill-all-local-variables *has* to be removed since it undoes the previous
call to special-mode).

>    (setq truncate-lines t)
> -  (setq buffer-read-only t)
> -  (setq major-mode 'bookmark-bmenu-mode)
> -  (setq mode-name "Bookmark Menu")
>    (run-mode-hooks 'bookmark-bmenu-mode-hook))

Why remove (setq buffer-read-only t) ?
And why not remove the run-mode-hooks?

> === modified file 'lisp/doc-view.el'
> --- lisp/doc-view.el	2010-07-14 15:57:54 +0000
> +++ lisp/doc-view.el	2010-08-29 15:08:39 +0000
> @@ -333,7 +333,6 @@
>      (define-key map (kbd "C-c C-t")   'doc-view-open-text)
>      ;; Reconvert the current document.  Don't just use revert-buffer
>      ;; because that resets the scale factor, the page number, ...
> -    (define-key map (kbd "g")         'doc-view-revert-buffer)

If you remove this line, you need to remove the above comment as well
(after making sure that the problem it mentions don't apply any more).

> +    (set (make-local-variable 'revert-buffer-function) 'doc-view-revert-buffer)
>      (run-mode-hooks 'doc-view-mode-hook)))

Here as well you should remove the run-mode-hooks.

>  ;;;###autoload
> -(defun image-mode ()
> +(define-derived-mode image-mode special-mode "Image"
>    "Major mode for image files.
>  You can use \\<image-mode-map>\\[image-toggle-display]
>  to toggle between display as an image and display as text."
> -  (interactive)
>    (condition-case err
>        (progn
>  	(unless (display-images-p)
>  	  (error "Display does not support images"))
 
>  	(kill-all-local-variables)
> -	(setq major-mode 'image-mode)

IIRC define-derived-mode sets major-mode before running the body of the
macro, so when you call `error' above, `major-mode' and `mode-name' have
already been set and need to be reverted.

> -(defun occur-mode ()
> +(define-derived-mode occur-mode special-mode "Occur"
>    "Major mode for output from \\[occur].
>  \\<occur-mode-map>Move point to one of the items in this buffer, then use
>  \\[occur-mode-goto-occurrence] to go to the occurrence that the item refers to.
>  Alternatively, click \\[occur-mode-mouse-goto] on an item to go to it.
 
>  \\{occur-mode-map}"
> -  (interactive)
>    (kill-all-local-variables)
>    (use-local-map occur-mode-map)

These last two should disappear as mentioned earlier.


        Stefan