Re: Refreshing Info nodes

[Wojciech Meyer]

Again, thanks a lot for spending time on it. In the meantime I will fix
these problems, and send you back the new one.

Thanks,
Wojciech

Stefan Monnier <address@hidden> writes:

>> 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