bug#5513: Archives disregard special mode-class

bug#5513: Archives disregard special mode-class

[Juri Linkov]

Visiting a file from an archive activates view-mode even when its major
mode has the property `mode-class' equal to `special'.  For instance,
visiting a .jar archive from .xpi archives or from .tar archives.
(See also bug#4896 for more information.)

This patch takes into account the special mode-class property
and doesn't activate view-mode in this case:

=== modified file 'lisp/arc-mode.el'
--- lisp/arc-mode.el	2010-02-01 23:02:47 +0000
+++ lisp/arc-mode.el	2010-02-02 23:10:14 +0000
@@ -1057,8 +1057,16 @@ (defun archive-extract (&optional other-
 	(archive-maybe-update t))
       (or (not (buffer-name buffer))
           (cond
-           (view-p (view-buffer
-		    buffer (and just-created 'kill-buffer-if-not-modified)))
+           (view-p
+	    ;; FIXME: code duplicated from `view-file', perhaps the test for special
+	    ;; mode-class should be moved from `view-file' to `view-buffer'.
+	    (if (eq (with-current-buffer buffer
+		      (get major-mode 'mode-class))
+		    'special)
+		(progn
+		  (switch-to-buffer buffer)
+		  (message "Not using View mode because the major mode is special"))
+	      (view-buffer buffer (and just-created 'kill-buffer-if-not-modified))))
            ((eq other-window-p 'display) (display-buffer buffer))
            (other-window-p (switch-to-buffer-other-window buffer))
            (t (switch-to-buffer buffer))))))

=== modified file 'lisp/tar-mode.el'
--- lisp/tar-mode.el	2010-01-13 08:35:10 +0000
+++ lisp/tar-mode.el	2010-01-31 20:06:55 +0000
@@ -852,14 +852,23 @@ (defun tar-extract (&optional other-wind
           (set (make-local-variable 'tar-superior-descriptor) descriptor)
           (setq buffer-read-only read-only-p)
           (tar-subfile-mode 1)))
-      (if view-p
-	  (view-buffer
-	   buffer (and just-created 'kill-buffer-if-not-modified))
-	(if (eq other-window-p 'display)
-	    (display-buffer buffer)
-	  (if other-window-p
-	      (switch-to-buffer-other-window buffer)
-	    (switch-to-buffer buffer)))))))
+      (cond
+       (view-p
+	;; FIXME: code duplicated from `view-file', perhaps the test for special
+	;; mode-class should be moved from `view-file' to `view-buffer'.
+	(if (eq (with-current-buffer buffer
+		  (get major-mode 'mode-class))
+		'special)
+	    (progn
+	      (switch-to-buffer buffer)
+	      (message "Not using View mode because the major mode is special"))
+	  (view-buffer buffer (and just-created 'kill-buffer-if-not-modified))))
+       ((eq other-window-p 'display)
+	(display-buffer buffer))
+       (other-window-p
+	(switch-to-buffer-other-window buffer))
+       (t
+	(switch-to-buffer buffer))))))
 
 
 (defun tar-extract-other-window ()

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#5513: Archives disregard special mode-class

[Stefan Monnier]

>     ;; FIXME: code duplicated from `view-file', perhaps the test for
>     ;; special mode-class should be moved from `view-file' to `view-buffer'.

Then at least move it to a new view-buffer-1 function.
But please don't introduce such code duplication.


        Stefan

bug#5513: Archives disregard special mode-class

[Juri Linkov]

>>     ;; FIXME: code duplicated from `view-file', perhaps the test for
>>     ;; special mode-class should be moved from `view-file' to `view-buffer'.
>
> Then at least move it to a new view-buffer-1 function.
> But please don't introduce such code duplication.

I think all `view-buffer' calls should respect the property `mode-class',
so I moved this code from `view-file' to `view-buffer'.  `view-buffer-1'
is a new function with the old content of `view-buffer' for the case when
some package would prefer to ignore `mode-class' for some reasons.

I hesitate to install this patch now because it changes the behavior of
`view-file' for a non-critical bug.

=== modified file 'lisp/view.el'
--- lisp/view.el	2010-01-13 08:35:10 +0000
+++ lisp/view.el	2010-02-04 00:05:22 +0000
@@ -263,13 +263,7 @@ (defun view-file (file)
   (unless (file-exists-p file) (error "%s does not exist" file))
   (let ((had-a-buf (get-file-buffer file))
 	(buffer (find-file-noselect file)))
-    (if (eq (with-current-buffer buffer
-	      (get major-mode 'mode-class))
-	    'special)
-	(progn
-	  (switch-to-buffer buffer)
-	  (message "Not using View mode because the major mode is special"))
-      (view-buffer buffer (and (not had-a-buf) 'kill-buffer-if-not-modified)))))
+    (view-buffer buffer (and (not had-a-buf) 'kill-buffer-if-not-modified))))
 
 ;;;###autoload
 (defun view-file-other-window (file)
@@ -335,6 +329,15 @@ (defun view-buffer (buffer &optional exi
 Exiting View mode will then discard the user's edits.  Setting
 EXIT-ACTION to `kill-buffer-if-not-modified' avoids this."
   (interactive "bView buffer: ")
+  (if (eq (with-current-buffer buffer
+	    (get major-mode 'mode-class))
+	  'special)
+      (progn
+	(switch-to-buffer buffer)
+	(message "Not using View mode because the major mode is special"))
+    (view-buffer-1 buffer exit-action)))
+
+(defun view-buffer-1 (buffer &optional exit-action)
   (let ((undo-window (list (window-buffer) (window-start) (window-point))))
     (switch-to-buffer buffer)
     (view-mode-enter (cons (selected-window) (cons nil undo-window))

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#5513: Archives disregard special mode-class

[Stefan Monnier]

> I think all `view-buffer' calls should respect the property `mode-class',
> so I moved this code from `view-file' to `view-buffer'.  `view-buffer-1'
> is a new function with the old content of `view-buffer' for the case when
> some package would prefer to ignore `mode-class' for some reasons.

> I hesitate to install this patch now because it changes the behavior of
> `view-file' for a non-critical bug.

Indeed, in this form I don't think it's acceptable for `trunk'.
If you switch the names around, OTOH it should be safe.


        Stefan

bug#5513: Archives disregard special mode-class

[Juri Linkov]

>> I think all `view-buffer' calls should respect the property `mode-class',
>> so I moved this code from `view-file' to `view-buffer'.  `view-buffer-1'
>> is a new function with the old content of `view-buffer' for the case when
>> some package would prefer to ignore `mode-class' for some reasons.
>
>> I hesitate to install this patch now because it changes the behavior of
>> `view-file' for a non-critical bug.
>
> Indeed, in this form I don't think it's acceptable for `trunk'.
> If you switch the names around, OTOH it should be safe.

I think in the long run the correct behavior of `view-buffer' should be to
check `mode-class'.  Instead of switching the behavior after the release,
we could wait and install the final version of `view-buffer' (that checks for
`mode-class') after feature freeze (or install now to the `pending' branch).

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#5513: Archives disregard special mode-class

[Stefan Monnier]

> I think in the long run the correct behavior of `view-buffer' should be to
> check `mode-class'.  Instead of switching the behavior after the release,
> we could wait and install the final version of `view-buffer' (that checks for
> `mode-class') after feature freeze (or install now to the `pending' branch).

Fine by me,


        Stefan

bug#5513: Archives disregard special mode-class

[Juri Linkov]

>> I think in the long run the correct behavior of `view-buffer' should be to
>> check `mode-class'.  Instead of switching the behavior after the release,
>> we could wait and install the final version of `view-buffer' (that checks for
>> `mode-class') after feature freeze (or install now to the `pending' branch).
>
> Fine by me,

Done.

-- 
Juri Linkov
http://www.jurta.org/emacs/