bug#7252: hexl-revert-buffer

bug#7252: hexl-revert-buffer

[Daiki Ueno]

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

If I open a ZIP file with M-x hexl-find-file and then M-x revert-buffer,
I see hexified file list generated by arc-mode, instead of raw ZIP data.

This behavior is surprising for me and it is cumbersome to do M-x
kill-buffer and M-x hexl-find-file each time.

How about adding M-x hexl-revert-buffer?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: hexl-revert-buffer.diff --]
[-- Type: text/x-patch, Size: 950 bytes --]

=== modified file 'lisp/hexl.el'
--- lisp/hexl.el	2010-10-10 23:12:30 +0000
+++ lisp/hexl.el	2010-10-20 05:46:47 +0000
@@ -189,6 +189,7 @@
     (define-key map "\C-x\C-p" 'undefined)
     (define-key map "\C-x\C-s" 'hexl-save-buffer)
     (define-key map "\C-x\C-t" 'undefined)
+    (define-key map "\C-xg" 'hexl-revert-buffer)
     map))
 
 ;; Variable declarations for suppressing warnings from the byte-compiler.
@@ -464,6 +465,16 @@
   (if (not (eq major-mode 'hexl-mode))
       (hexl-mode)))
 
+(defun hexl-revert-buffer ()
+  "Revert the buffer previously opened with `hexl-find-file'."
+  (interactive)
+  (if (eq major-mode 'hexl-mode)
+      (let ((coding-system-for-read 'no-conversion))
+	(revert-buffer nil nil t)
+	(setq major-mode 'fundamental-mode)
+	(hexl-mode))
+    (call-interactively #'revert-buffer)))
+
 (defun hexl-mode-exit (&optional arg)
   "Exit Hexl mode, returning to previous mode.
 With arg, don't unhexlify buffer."


[-- Attachment #3: Type: text/plain, Size: 25 bytes --]


Regards,
-- 
Daiki Ueno

bug#7252: hexl-revert-buffer

[Stefan Monnier]

> If I open a ZIP file with M-x hexl-find-file and then M-x revert-buffer,
> I see hexified file list generated by arc-mode, instead of raw ZIP data.

> This behavior is surprising for me and it is cumbersome to do M-x
> kill-buffer and M-x hexl-find-file each time.

> How about adding M-x hexl-revert-buffer?

No, we should tweak revert-buffer-function instead.

> +  (if (eq major-mode 'hexl-mode)

Use derived-mode-p rather than looking at major-mode.

> +      (let ((coding-system-for-read 'no-conversion))
> +	(revert-buffer nil nil t)
> +	(setq major-mode 'fundamental-mode)

Why is this setq needed (IOW please add a comment explaining why it's
needed).


        Stefan

bug#7252: hexl-revert-buffer

[Daiki Ueno]

[-- Attachment #1: Type: text/plain, Size: 447 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> If I open a ZIP file with M-x hexl-find-file and then M-x revert-buffer,
>> I see hexified file list generated by arc-mode, instead of raw ZIP data.
>
>> This behavior is surprising for me and it is cumbersome to do M-x
>> kill-buffer and M-x hexl-find-file each time.
>
>> How about adding M-x hexl-revert-buffer?
>
> No, we should tweak revert-buffer-function instead.

Ok, how about this?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: hexl-revert-buffer.diff --]
[-- Type: text/x-patch, Size: 3161 bytes --]

=== modified file 'lisp/hexl.el'
--- lisp/hexl.el	2010-10-10 23:12:30 +0000
+++ lisp/hexl.el	2010-10-21 01:51:54 +0000
@@ -212,6 +212,7 @@
 (defvar hexl-mode-old-syntax-table)
 (defvar hexl-mode-old-font-lock-keywords)
 (defvar hexl-mode-old-eldoc-documentation-function)
+(defvar hexl-mode-old-revert-buffer-function)
 
 (defvar hexl-ascii-overlay nil
   "Overlay used to highlight ASCII element corresponding to current point.")
@@ -373,10 +374,9 @@
     (setq hexl-mode-old-font-lock-keywords font-lock-defaults)
     (setq font-lock-defaults '(hexl-font-lock-keywords t))
 
-    ;; Add hooks to rehexlify or dehexlify on various events.
-    (add-hook 'before-revert-hook 'hexl-before-revert-hook nil t)
-    (add-hook 'after-revert-hook 'hexl-after-revert-hook nil t)
-
+    (make-local-variable 'hexl-mode-old-revert-buffer-function)
+    (setq hexl-mode-old-revert-buffer-function revert-buffer-function)
+    (setq revert-buffer-function 'hexl-revert-buffer-function)
     (add-hook 'change-major-mode-hook 'hexl-maybe-dehexlify-buffer nil t)
 
     ;; Set a callback function for eldoc.
@@ -413,12 +413,6 @@
     (let ((isearch-search-fun-function nil))
       (isearch-search-fun))))
 
-(defun hexl-before-revert-hook ()
-  (remove-hook 'change-major-mode-hook 'hexl-maybe-dehexlify-buffer t))
-
-(defun hexl-after-revert-hook ()
-  (hexl-mode))
-
 (defvar hexl-in-save-buffer nil)
 
 (defun hexl-save-buffer ()
@@ -464,6 +458,23 @@
   (if (not (eq major-mode 'hexl-mode))
       (hexl-mode)))
 
+(defun hexl-revert-buffer-function (ignore-auto noconfirm)
+  (let ((coding-system-for-read 'no-conversion)
+	revert-buffer-function)
+    ;; Call the original `revert-buffer' without code conversion; also
+    ;; prevent it changes the major mode to normal-mode, which calls
+    ;; `set-auto-mode'.
+    (revert-buffer nil nil t)
+    ;; A couple of hacks are necessary here:
+    ;; 1. change the major-mode to one other than hexl-mode since the
+    ;; function `hexl-mode' does nothing if the current major-mode is
+    ;; already hexl-mode.
+    ;; 2. reset change-major-mode-hook in case that `hexl-mode'
+    ;; previously add `hexl-maybe-dehexlify-buffer' to it.
+    (remove-hook 'change-major-mode-hook 'hexl-maybe-dehexlify-buffer t)
+    (setq major-mode 'fundamental-mode)
+    (hexl-mode)))
+
 (defun hexl-mode-exit (&optional arg)
   "Exit Hexl mode, returning to previous mode.
 With arg, don't unhexlify buffer."
@@ -483,8 +494,6 @@
 	  (or (bobp) (setq original-point (1+ original-point))))
 	(goto-char original-point)))
 
-  (remove-hook 'before-revert-hook 'hexl-before-revert-hook t)
-  (remove-hook 'after-revert-hook 'hexl-after-revert-hook t)
   (remove-hook 'change-major-mode-hook 'hexl-maybe-dehexlify-buffer t)
   (remove-hook 'post-command-hook 'hexl-follow-ascii-find t)
   (setq hexl-ascii-overlay nil)
@@ -512,6 +521,7 @@
   (set-syntax-table hexl-mode-old-syntax-table)
   (setq font-lock-defaults hexl-mode-old-font-lock-keywords)
   (setq major-mode hexl-mode-old-major-mode)
+  (setq revert-buffer-function hexl-mode-old-revert-buffer-function)
   (force-mode-line-update))
 
 (defun hexl-maybe-dehexlify-buffer ()


[-- Attachment #3: Type: text/plain, Size: 25 bytes --]


Regards,
-- 
Daiki Ueno

bug#7252: hexl-revert-buffer

[Stefan Monnier]

>> No, we should tweak revert-buffer-function instead.
> Ok, how about this?

Looks good (even better than I thought since I hadn't noticed the
*-revert-hooks).  Assuming you've tested it, please install it.


        Stefan

bug#7252: hexl-revert-buffer

[Daiki Ueno]

Done as r102027.

Regards,
-- 
Daiki Ueno

bug#7252: hexl-revert-buffer

[Stefan Monnier]

> Done as r102027.

Thank you.

Actually, I see now that, while your patch is going in the right
direction, the problem is deeper.  E.g. M-x find-file-literally RET
foo.tar.gz RET does the right thing but M-x revert-buffer RET in it
ignores the "literally" part and gives you a tar-mode view of the file.
I.e. revert-buffer should probably pay attention to the
`find-file-literally' variable, and hexl-mode should then make use of it
as well.


        Stefan

bug#7252: hexl-revert-buffer

[Daiki Ueno]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Actually, I see now that, while your patch is going in the right
> direction, the problem is deeper.  E.g. M-x find-file-literally RET
> foo.tar.gz RET does the right thing but M-x revert-buffer RET in it
> ignores the "literally" part and gives you a tar-mode view of the file.

Right, but my personal feeling is that typical users would not expect
such a "literally"-ness of revert-buffer when the file is opened with
M-x find-file-literally, unlike with M-x hexl-find-file.

With M-x find-file-literally user's intension would be to quick look at
the raw content.  In that case he might do M-x revert-buffer to "go back
to the normal view of that file" and expect auto-mode conversion.

However, with M-x hexl-find-file user's intension would be to analyze
the file format or edit the raw content.  In that case auto-mode
conversion is definitely unwanted.

Perhaps it might be better to bring this up in emacs-devel.

Regards,
-- 
Daiki Ueno