[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Refreshing Info nodes
|
From: |
Wojciech Meyer |
|
Subject: |
Re: Refreshing Info nodes |
|
Date: |
Mon, 13 Sep 2010 02:34:05 +0100 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 |
Stefan Monnier <address@hidden> writes:
Thanks for it, below is my update:
>> 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.
Done. I am investigating possibility of using swap-buffer with
archive-mode. This is not done yet, so you might consider not applying
diffs to `arc-mode.el' (however I am planning to fix it ASAP too..)
BTW: Maybe we should think about merging (at least reusing bits & pieces
and making user interface more consistent) tar-mode <-> archive-mode?
>
>> -(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).
Done.
>
>> (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?
Done.
>
>> === 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).
>
Done. However for some reasons I am not able to revert the buffer with
"g" (and not sure why, AFAIK I could do it before, investigating)
That's why I've left "g" as 'revert-buffer.
BTW:
(let* ((prev-major-mode (if (eq major-mode 'doc-view-mode)
doc-view-previous-major-mode
major-mode)))
(set (make-local-variable 'doc-view-previous-major-mode)
prev-major-mode))
Here is code sensitive on previous major mode, please see comments
below.
Also, there is some problem with reverting itself:
it shows error message: `doc-view-revert-buffer: Variable binding depth
exceeds max-specpdl-size'. Did anybody notice something similar? (emacs
-Q, bzr trunk rev 101406).
>> + (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.
This could be accomplished by either:
- change to `derived.el', saving previous mode (this might be useful),
- change to `derived.el' by being reactive on exception (which is
valuable anyway, but there should be a way of telling to re-throw it).
- changing `image-mode' to something else and wrapping the rest into
`defun' (ad-hoc solution, which I like at least)
- having a flag that body should be executed before or after the body
generated by `define-derived-mode', or having a key, similar to
defadvice, (I wonder if EIEIO could help doing all of this..).
Ideally in case of saving previous mode we should have a stack for this
(even better, stack with functions that restore previous state, but this
might be an overkill).
I left it alone for time being, as the only drawback that user will
need to revert the mode by herself.
>
>> -(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.
>
>
Done.
BTW: Shall we have a special keyword for `derived-mode' that could possibly put
together
default keyboard-map with mode (or allow user to use existing
keyboard-map), that would make developing new major modes more
consistent:
(define-derived-mode my-major-mode special-mode
:keymap
(("n" . 'my-next-page)
("C-s" . 'my-search))
...)
or
(define-derived-mode my-major-mode special-mode
:keymap 'my-major-mode-map
...)
AFAIK there is `configuration by convention' (post-fix `-map') but IMHO
it would be nicer if the keymaps could be defined in the body of the
macro. (and possibly merged with derive-mode-set-keymap). Maybe even we
can consider, generating a separate function for user init code (the
body of `define-derived-mode').
As you notice I also think that defining key-maps should be easier.
Generally maybe we should do what's been done with `easy-menu' for maps.
For convenience I am attaching set of patches each file separately, as
they are independent.
> Stefan
Thanks,
Wojciech
=== modified file 'lisp/arc-mode.el'
--- lisp/arc-mode.el 2010-07-10 18:52:53 +0000
+++ lisp/arc-mode.el 2010-09-13 00:49:20 +0000
@@ -341,7 +341,6 @@
(defvar archive-mode-map
(let ((map (make-keymap)))
(suppress-keymap map)
- (define-key map " " 'archive-next-line)
(define-key map "a" 'archive-alternate-display)
;;(define-key map "c" 'archive-copy)
(define-key map "d" 'archive-flag-deleted)
@@ -349,15 +348,12 @@
(define-key map "e" 'archive-extract)
(define-key map "f" 'archive-extract)
(define-key map "\C-m" 'archive-extract)
- (define-key map "g" 'revert-buffer)
- (define-key map "h" 'describe-mode)
(define-key map "m" 'archive-mark)
(define-key map "n" 'archive-next-line)
(define-key map "\C-n" 'archive-next-line)
(define-key map [down] 'archive-next-line)
(define-key map "o" 'archive-extract-other-window)
(define-key map "p" 'archive-previous-line)
- (define-key map "q" 'quit-window)
(define-key map "\C-p" 'archive-previous-line)
(define-key map [up] 'archive-previous-line)
(define-key map "r" 'archive-rename-entry)
@@ -632,31 +628,16 @@
(error "Entry is not a regular member of the archive"))))
(if (not noerror)
(error "Line does not describe a member of the archive")))))
-;; -------------------------------------------------------------------------
-;;; Section: the mode definition
-
-;;;###autoload
-(defun archive-mode (&optional force)
- "Major mode for viewing an archive file in a dired-like way.
-You can move around using the usual cursor motion commands.
-Letters no longer insert themselves.
-Type `e' to pull a file out of the archive and into its own buffer;
-or click mouse-2 on the file's line in the archive mode buffer.
-
-If you edit a sub-file of this archive (as with the `e' command) and
-save it, the contents of that buffer will be saved back into the
-archive.
-
-\\{archive-mode-map}"
- ;; This is not interactive because you shouldn't be turning this
- ;; mode on and off. You can corrupt things that way.
+
+(defun archive-setup-mode (&optional force)
+ "Wrap all the mode setup code into separate function callable from
+the package code."
(if (zerop (buffer-size))
;; At present we cannot create archives from scratch
(funcall (or (default-value 'major-mode) 'fundamental-mode))
(if (and (not force) archive-files) nil
(let* ((type (archive-find-type))
(typename (capitalize (symbol-name type))))
- (kill-all-local-variables)
(make-local-variable 'archive-subtype)
(setq archive-subtype type)
@@ -701,8 +682,7 @@
(setq major-mode 'archive-mode)
(setq mode-name (concat typename "-Archive"))
;; Run archive-foo-mode-hook and archive-mode-hook
- (run-mode-hooks (archive-name "mode-hook") 'archive-mode-hook)
- (use-local-map archive-mode-map))
+ (run-mode-hooks (archive-name "mode-hook") 'archive-mode-hook))
(make-local-variable 'archive-proper-file-start)
(make-local-variable 'archive-file-list-start)
@@ -717,6 +697,30 @@
(archive-summarize nil)
(setq buffer-read-only t))))
+;; -------------------------------------------------------------------------
+;;; Section: the mode definition
+
+;;;###autoload
+(define-derived-mode archive-mode special-mode "Archive mode"
+ "Major mode for viewing an archive file in a dired-like way.
+You can move around using the usual cursor motion commands.
+Letters no longer insert themselves.
+Type `e' to pull a file out of the archive and into its own buffer;
+or click mouse-2 on the file's line in the archive mode buffer.
+
+If you edit a sub-file of this archive (as with the `e' command) and
+save it, the contents of that buffer will be saved back into the
+archive.
+
+\\{archive-mode-map}"
+ ;; 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.
+ ;;
+ (archive-setup-mode))
+
+
;; Archive mode is suitable only for specially formatted data.
(put 'archive-mode 'mode-class 'special)
@@ -888,7 +892,7 @@
(setq archive-files nil)
(erase-buffer)
(insert-file-contents name)
- (archive-mode t)
+ (archive-setup-mode t)
(goto-char archive-file-list-start)
(archive-next-line lno))
(archive-delete-local name)
@@ -1396,7 +1400,7 @@
(let ((revert-buffer-function nil)
(coding-system-for-read 'no-conversion))
(revert-buffer t t))
- (archive-mode)
+ (archive-setup-mode)
(goto-char archive-file-list-start)
(archive-next-line no)))
=== modified file 'lisp/bookmark.el'
--- lisp/bookmark.el 2010-07-14 19:09:28 +0000
+++ lisp/bookmark.el 2010-09-13 01:08:20 +0000
@@ -853,29 +853,22 @@
(define-key map "\C-c\C-c" 'bookmark-send-edited-annotation)
map)
"Keymap for editing an annotation of a bookmark.")
-
-
-(defun bookmark-edit-annotation-mode (bookmark)
+
+(define-derived-mode bookmark-edit-annotation-mode nil "Edit Bookmark
Annotation"
"Mode for editing the annotation of bookmark BOOKMARK.
When you have finished composing, type \\[bookmark-send-annotation].
BOOKMARK is a bookmark name (a string) or a bookmark record.
\\{bookmark-edit-annotation-mode-map}"
- (interactive)
- (kill-all-local-variables)
+ (use-local-map bookmark-edit-annotation-mode-map)
(make-local-variable 'bookmark-annotation-name)
- (setq bookmark-annotation-name bookmark)
- (use-local-map bookmark-edit-annotation-mode-map)
- (setq major-mode 'bookmark-edit-annotation-mode
- mode-name "Edit Bookmark Annotation")
(insert (funcall bookmark-edit-annotation-text-func bookmark))
(let ((annotation (bookmark-get-annotation bookmark)))
(if (and annotation (not (string-equal annotation "")))
(insert annotation)))
(run-mode-hooks 'text-mode-hook))
-
(defun bookmark-send-edited-annotation ()
"Use buffer contents as annotation for a bookmark.
Lines beginning with `#' are ignored."
@@ -901,8 +894,8 @@
"Pop up a buffer for editing bookmark BOOKMARK's annotation.
BOOKMARK is a bookmark name (a string) or a bookmark record."
(pop-to-buffer (generate-new-buffer-name "*Bookmark Annotation Compose*"))
- (bookmark-edit-annotation-mode bookmark))
-
+ (bookmark-edit-annotation-mode)
+ (setq bookmark-annotation-name bookmark))
(defun bookmark-insert-current-bookmark ()
"Insert into the bookmark name currently being set the value of
@@ -1499,7 +1492,6 @@
(defvar bookmark-bmenu-mode-map
(let ((map (make-keymap)))
(suppress-keymap map t)
- (define-key map "q" 'quit-window)
(define-key map "v" 'bookmark-bmenu-select)
(define-key map "w" 'bookmark-bmenu-locate)
(define-key map "2" 'bookmark-bmenu-2-window)
@@ -1515,11 +1507,9 @@
(define-key map "\C-d" 'bookmark-bmenu-delete-backwards)
(define-key map "x" 'bookmark-bmenu-execute-deletions)
(define-key map "d" 'bookmark-bmenu-delete)
- (define-key map " " 'next-line)
(define-key map "n" 'next-line)
(define-key map "p" 'previous-line)
(define-key map "\177" 'bookmark-bmenu-backup-unmark)
- (define-key map "?" 'describe-mode)
(define-key map "u" 'bookmark-bmenu-unmark)
(define-key map "m" 'bookmark-bmenu-mark)
(define-key map "l" 'bookmark-bmenu-load)
@@ -1609,7 +1599,7 @@
-(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.
@@ -1642,13 +1632,8 @@
in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all
bookmarks in another buffer.
\\[bookmark-bmenu-edit-annotation] -- edit the annotation for the current
bookmark."
- (kill-all-local-variables)
- (use-local-map bookmark-bmenu-mode-map)
- (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))
+ (setq truncate-lines t))
(defun bookmark-bmenu-toggle-filenames (&optional show)
=== modified file 'lisp/doc-view.el'
--- lisp/doc-view.el 2010-07-14 15:57:54 +0000
+++ lisp/doc-view.el 2010-09-13 00:13:30 +0000
@@ -331,9 +331,7 @@
(define-key map (kbd "C-c C-c") 'doc-view-toggle-display)
;; Open a new buffer with doc's text contents
(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)
+ ;; Reconvert the current document.
(define-key map (kbd "r") 'doc-view-revert-buffer)
map)
"Keymap used by `doc-view-mode' when displaying a doc as a set of images.")
@@ -1207,7 +1205,7 @@
l))
;;;###autoload
-(defun doc-view-mode ()
+(define-derived-mode doc-view-mode special-mode "Doc"
"Major mode in DocView buffers.
DocView Mode is an Emacs document viewer. It displays PDF, PS
@@ -1216,7 +1214,6 @@
You can use \\<doc-view-mode-map>\\[doc-view-toggle-display] to
toggle between displaying the document or editing it as text.
\\{doc-view-mode-map}"
- (interactive)
(if (= (point-min) (point-max))
;; The doc is empty or doesn't exist at all, so fallback to
@@ -1229,7 +1226,6 @@
(let* ((prev-major-mode (if (eq major-mode 'doc-view-mode)
doc-view-previous-major-mode
major-mode)))
- (kill-all-local-variables)
(set (make-local-variable 'doc-view-previous-major-mode)
prev-major-mode))
;; Figure out the document type.
@@ -1307,20 +1303,17 @@
(set (make-local-variable 'mwheel-scroll-down-function)
'doc-view-scroll-down-or-previous-page)
(set (make-local-variable 'cursor-type) nil)
- (use-local-map doc-view-mode-map)
(set (make-local-variable 'after-revert-hook) 'doc-view-reconvert-doc)
(set (make-local-variable 'bookmark-make-record-function)
'doc-view-bookmark-make-record)
- (setq mode-name "DocView"
- buffer-read-only t
- major-mode 'doc-view-mode)
+ (setq buffer-read-only t)
(doc-view-initiate-display)
;; Switch off view-mode explicitly, because doc-view-mode is the
;; canonical view mode for PDF/PS/DVI files. This could be
;; switched on automatically depending on the value of
;; `view-read-only'.
(set (make-local-variable 'view-read-only) nil)
- (run-mode-hooks 'doc-view-mode-hook)))
+ (set (make-local-variable 'revert-buffer-function)
'doc-view-revert-buffer)))
;;;###autoload
(define-minor-mode doc-view-minor-mode
=== modified file 'lisp/image-mode.el'
--- lisp/image-mode.el 2010-08-29 16:17:13 +0000
+++ lisp/image-mode.el 2010-09-13 00:10:56 +0000
@@ -315,9 +315,7 @@
(defvar image-mode-map
(let ((map (make-sparse-keymap)))
(suppress-keymap map)
- (define-key map "q" 'quit-window)
(define-key map "\C-c\C-c" 'image-toggle-display)
- (define-key map (kbd "SPC") 'image-scroll-up)
(define-key map (kbd "DEL") 'image-scroll-down)
(define-key map [remap forward-char] 'image-forward-hscroll)
(define-key map [remap backward-char] 'image-backward-hscroll)
@@ -347,19 +345,16 @@
(put 'image-mode 'mode-class 'special)
;;;###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)
-
(if (not (image-get-display-property))
(progn
(image-toggle-display-image)
@@ -372,7 +367,6 @@
image-type (plist-get (cdr (image-get-display-property))
:type)))
(setq mode-name (if image-type (format "Image[%s]" image-type) "Image"))
- (use-local-map image-mode-map)
;; Use our own bookmarking function for images.
(set (make-local-variable 'bookmark-make-record-function)
=== modified file 'lisp/replace.el'
--- lisp/replace.el 2010-08-29 16:17:13 +0000
+++ lisp/replace.el 2010-09-12 22:12:21 +0000
@@ -767,8 +767,6 @@
(define-key map "\M-p" 'occur-prev)
(define-key map "r" 'occur-rename-buffer)
(define-key map "c" 'clone-buffer)
- (define-key map "g" 'revert-buffer)
- (define-key map "q" 'quit-window)
(define-key map "z" 'kill-this-buffer)
(define-key map "\C-c\C-f" 'next-error-follow-minor-mode)
(define-key map [menu-bar] (make-sparse-keymap))
@@ -835,18 +833,13 @@
:group 'matching)
(put 'occur-mode 'mode-class 'special)
-(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)
- (setq major-mode 'occur-mode)
- (setq mode-name "Occur")
(set (make-local-variable 'revert-buffer-function) 'occur-revert-function)
(make-local-variable 'occur-revert-arguments)
(add-hook 'change-major-mode-hook 'font-lock-defontify nil t)
=== modified file 'lisp/tar-mode.el'
--- lisp/tar-mode.el 2010-05-03 02:29:46 +0000
+++ lisp/tar-mode.el 2010-08-28 19:52:48 +0000
@@ -524,7 +524,6 @@
(defvar tar-mode-map
(let ((map (make-keymap)))
(suppress-keymap map)
- (define-key map " " 'tar-next-line)
(define-key map "C" 'tar-copy)
(define-key map "d" 'tar-flag-deleted)
(define-key map "\^D" 'tar-flag-deleted)
@@ -532,14 +531,12 @@
(define-key map "f" 'tar-extract)
(define-key map "\C-m" 'tar-extract)
(define-key map [mouse-2] 'tar-mouse-extract)
- (define-key map "g" 'revert-buffer)
(define-key map "h" 'describe-mode)
(define-key map "n" 'tar-next-line)
(define-key map "\^N" 'tar-next-line)
(define-key map [down] 'tar-next-line)
(define-key map "o" 'tar-extract-other-window)
(define-key map "p" 'tar-previous-line)
- (define-key map "q" 'quit-window)
(define-key map "\^P" 'tar-previous-line)
(define-key map [up] 'tar-previous-line)
(define-key map "R" 'tar-rename-entry)
@@ -615,7 +612,7 @@
(if (buffer-live-p tar-data-buffer) (kill-buffer tar-data-buffer)))
;;;###autoload
-(define-derived-mode tar-mode nil "Tar"
+(define-derived-mode tar-mode special-mode "Tar"
"Major mode for viewing a tar file as a dired-like listing of its contents.
You can move around using the usual cursor motion commands.
Letters no longer insert themselves.