unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
@ 2016-01-24  6:30 Alexander Kuleshov
  2016-01-24 14:48 ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander Kuleshov @ 2016-01-24  6:30 UTC (permalink / raw)
  To: 22453; +Cc: Alexander Kuleshov

We can easily switch to text mode from the image mode by the
pressing of C-c C-c. But sometimes, it is more useful to open
an image in hex format. This patch provides new keybinding
for the image mode - C-c C-x which works like C-c C-c, but
executes switch to the hexl-mode. Like switching to text mode,
switching to hex mode supports switching back to the image-mode.

The patch contains following changes:

* lisp/image-mode.el: adapted comments at the top of file;
* lisp/image-mode.el: added new menu item to the Image menu:
'Show as hex';
* lisp/image-mode.el: added description of the new keybinding
to the image-mode help;
* lisp/image-mode.el: all help messages in minibuffer adapted
to new keybinding;
* lisp/image-mode.el: image-toggle-hex-display function added
which checks current mode and switch into text or image mode
depends on it;
* lisp/image-mode.el: add image-mode-to-text helper for 
image-toggle-as-hex and image-toggle-as-text functions. It
is built on top of image-toggle-as-text function and used
for auto-mode-alist related preparation before switching
into text or hexl mode.

Patch tested on top of emacs-25 branch on fedora 23 with all
Linux related configured options.
---
 lisp/image-mode.el | 96 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 71 insertions(+), 25 deletions(-)

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index e549b49..5ee00bb 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -24,8 +24,8 @@
 ;;; Commentary:
 
 ;; Defines a major mode for visiting image files
-;; that allows conversion between viewing the text of the file
-;; and viewing the file as an image.  Viewing the image
+;; that allows conversion between viewing the text of the file,
+;; hex of the file and viewing the file as an image.  Viewing the image
 ;; works by putting a `display' text-property on the
 ;; image data, with the image-data still present underneath; if the
 ;; resulting buffer file is saved to another name it will correctly save
@@ -374,6 +374,7 @@ image-mode-map
   (let ((map (make-sparse-keymap)))
     (set-keymap-parent map special-mode-map)
     (define-key map "\C-c\C-c" 'image-toggle-display)
+    (define-key map "\C-c\C-x" 'image-toggle-hex-display)
     (define-key map (kbd "SPC")       'image-scroll-up)
     (define-key map (kbd "S-SPC")     'image-scroll-down)
     (define-key map (kbd "DEL")       'image-scroll-down)
@@ -406,6 +407,8 @@ image-mode-map
       '("Image"
 	["Show as Text" image-toggle-display :active t
 	 :help "Show image as text"]
+	["Show as Hex" image-toggle-hex-display :active t
+	 :help "Show image as hex"]
 	"--"
 	["Fit to Window Height" image-transform-fit-to-height
 	 :visible (eq image-type 'imagemagick)
@@ -480,6 +483,7 @@ image-mode-map
 (defvar image-minor-mode-map
   (let ((map (make-sparse-keymap)))
     (define-key map "\C-c\C-c" 'image-toggle-display)
+    (define-key map "\C-c\C-x" 'image-toggle-hex-display)
     map)
   "Mode keymap for `image-minor-mode'.")
 
@@ -490,8 +494,8 @@ bookmark-make-record-function
 ;;;###autoload
 (defun image-mode ()
   "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.
+You can use \\<image-mode-map>\\[image-toggle-display] or \\<image-mode-map>\\[image-toggle-hex-display]
+to toggle between display as an image and display as text or hex.
 
 Key bindings:
 \\{image-mode-map}"
@@ -530,7 +534,7 @@ image-mode
 	(run-mode-hooks 'image-mode-hook)
 	(let ((image (image-get-display-property))
 	      (msg1 (substitute-command-keys
-		     "Type \\[image-toggle-display] to view the image as "))
+		     "Type \\[image-toggle-display] or \\[image-toggle-hex-display] to view the image as "))
 	      animated)
 	  (cond
 	   ((null image)
@@ -559,7 +563,7 @@ image-mode
 ;;;			     (substitute-command-keys
 ;;;			      "\\[image-toggle-animation] to animate."))))
 	   (t
-	    (message "%s" (concat msg1 "text."))))))
+	    (message "%s" (concat msg1 "text or hex."))))))
 
     (error
      (image-mode-as-text)
@@ -585,19 +589,10 @@ image-minor-mode
       (add-hook 'change-major-mode-hook (lambda () (image-minor-mode -1)) nil t)))
 
 ;;;###autoload
-(defun image-mode-as-text ()
+(defun image-mode-to-text ()
   "Set a non-image mode as major mode in combination with image minor mode.
-A non-image major mode found from `auto-mode-alist' or Fundamental mode
-displays an image file as text.  `image-minor-mode' provides the key
-\\<image-mode-map>\\[image-toggle-display] to switch back to `image-mode'
-to display an image file as the actual image.
-
-You can use `image-mode-as-text' in `auto-mode-alist' when you want
-to display an image file as text initially.
-
-See commands `image-mode' and `image-minor-mode' for more information
-on these modes."
-  (interactive)
+A non-mage major mode found from `auto-mode-alist' or fundamental mode
+displays an image file as hex."
   ;; image-mode-as-text = normal-mode + image-minor-mode
   (let ((previous-image-type image-type)) ; preserve `image-type'
     (if image-mode-previous-major-mode
@@ -625,12 +620,49 @@ image-mode-as-text
     ;; Enable image minor mode with `C-c C-c'.
     (image-minor-mode 1)
     ;; Show the image file as text.
-    (image-toggle-display-text)
-    (message "%s" (concat
-		   (substitute-command-keys
-		    "Type \\[image-toggle-display] to view the image as ")
-		   (if (image-get-display-property)
-		       "text" "an image") "."))))
+    (image-toggle-display-text)))
+
+(defun image-mode-as-hex ()
+  "Set a non-image mode as major mode in combination with image minor mode.
+A non-mage major mode found from `auto-mode-alist' or fundamental mode
+displays an image file as hex.  `image-minor-mode' provides the key
+\\<image-mode-map>\\[image-toggle-hex-display] to switch back  to `image-mode'
+to display an image file as the actual image.
+
+You can use `image-mode-as-text' in `auto-mode-alist' when you wanto
+to display an image file as text initially.
+
+See commands `image-mode' and `image-minor-mode' for more information
+on these modes."
+  (interactive)
+  (image-mode-to-text)
+  ;; Turn on hexl-mode
+  (hexl-mode)
+  (message "%s" (concat
+                 (substitute-command-keys
+                  "Type \\[image-toggle-hex-display] or \\[image-toggle-display] to view the image as ")
+                 (if (image-get-display-property)
+                     "hex" "an image or text") ".")))
+
+(defun image-mode-as-text ()
+  "Set a non-image mode as major mode in combination with image minor mode.
+A non-image major mode found from `auto-mode-alist' or Fundamental mode
+displays an image file as text.  `image-minor-mode' provides the key
+\\<image-mode-map>\\[image-toggle-display] to switch back to `image-mode'
+to display an image file as the actual image.
+
+You can use `image-mode-as-text' in `auto-mode-alist' when you want
+to display an image file as text initially.
+
+See commands `image-mode' and `image-minor-mode' for more information
+on these modes."
+  (interactive)
+  (image-mode-to-text)
+  (message "%s" (concat
+                 (substitute-command-keys
+                  "Type \\[image-toggle-display] or \\[image-toggle-hex-display] to view the image as ")
+                 (if (image-get-display-property)
+                     "text" "an image or hex") ".")))
 
 (define-obsolete-function-alias 'image-mode-maybe 'image-mode "23.2")
 
@@ -725,6 +757,18 @@ image-toggle-display-image
     (if (called-interactively-p 'any)
 	(message "Repeat this command to go back to displaying the file as text"))))
 
+(defun image-toggle-hex-display ()
+  "Toggle between image and text display.
+If the current buffer is displaying an image file as an image,
+call `image-mode-as-hex' to switch to text.  Otherwise, display
+the image by calling `image-mode'."
+  (interactive)
+  (if (image-get-display-property)
+      (image-mode-as-hex)
+    (if (eq major-mode 'fundamental-mode)
+        (image-mode-as-hex)
+      (image-mode))))
+
 (defun image-toggle-display ()
   "Toggle between image and text display.
 If the current buffer is displaying an image file as an image,
@@ -733,7 +777,9 @@ image-toggle-display
   (interactive)
   (if (image-get-display-property)
       (image-mode-as-text)
-    (image-mode)))
+    (if (eq major-mode 'hexl-mode)
+        (image-mode-as-text)
+      (image-mode))))
 
 (defun image-kill-buffer ()
   "Kill the current buffer."
-- 
2.7.0.25.gfc10eb5






^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
  2016-01-24  6:30 bug#22453: [PATCH] Support for switching to hexl-mode from image mode Alexander Kuleshov
@ 2016-01-24 14:48 ` Eli Zaretskii
  2016-01-24 14:59   ` Alexander Kuleshov
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2016-01-24 14:48 UTC (permalink / raw)
  To: Alexander Kuleshov; +Cc: 22453

> From: Alexander Kuleshov <kuleshovmail@gmail.com>
> Date: Sun, 24 Jan 2016 12:30:36 +0600
> Cc: Alexander Kuleshov <kuleshovmail@gmail.com>
> 
> We can easily switch to text mode from the image mode by the
> pressing of C-c C-c. But sometimes, it is more useful to open
> an image in hex format. This patch provides new keybinding
> for the image mode - C-c C-x which works like C-c C-c, but
> executes switch to the hexl-mode. Like switching to text mode,
> switching to hex mode supports switching back to the image-mode.
> 
> The patch contains following changes:
> 
> * lisp/image-mode.el: adapted comments at the top of file;
> * lisp/image-mode.el: added new menu item to the Image menu:
> 'Show as hex';
> * lisp/image-mode.el: added description of the new keybinding
> to the image-mode help;
> * lisp/image-mode.el: all help messages in minibuffer adapted
> to new keybinding;
> * lisp/image-mode.el: image-toggle-hex-display function added
> which checks current mode and switch into text or image mode
> depends on it;
> * lisp/image-mode.el: add image-mode-to-text helper for 
> image-toggle-as-hex and image-toggle-as-text functions. It
> is built on top of image-toggle-as-text function and used
> for auto-mode-alist related preparation before switching
> into text or hexl mode.
> 
> Patch tested on top of emacs-25 branch on fedora 23 with all
> Linux related configured options.

Thanks.  This generally looks good.  A few comments:

  . This should go to master, not emacs-25, as it's a new feature.
  . Please provide a ChangeLog-style commit log message for the
    changes (some details are in CONTRIBUTE).
  . Please provide an update for the user manual, which describes
    "C-c C-c", to describe "C-x C-x" as well.

Some specific comments below.

> +(defun image-mode-as-hex ()
> +  "Set a non-image mode as major mode in combination with image minor mode.
> +A non-mage major mode found from `auto-mode-alist' or fundamental mode
> +displays an image file as hex.  `image-minor-mode' provides the key
> +\\<image-mode-map>\\[image-toggle-hex-display] to switch back  to `image-mode'
> +to display an image file as the actual image.
> +
> +You can use `image-mode-as-text' in `auto-mode-alist' when you wanto
                                                                  ^^^^^
A typo.

> +to display an image file as text initially.

Why does this sentence talk about text, when the mode is about hex
display?  Copy/paste mistake?

> +(defun image-toggle-hex-display ()
> +  "Toggle between image and text display.
> +If the current buffer is displaying an image file as an image,
> +call `image-mode-as-hex' to switch to text.  Otherwise, display
> +the image by calling `image-mode'."

It is best to avoid referring to another command in a doc string, if
you can avoid that.  In this case, the reference doesn't add any
useful information, so I think it should be deleted.

Last, but not least, I think this contribution is too large to accept
without legal paperwork.  I don't see your copyright assignment on
file; would you like me to send you the form so you could start the
paperwork rolling?

Thanks.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
  2016-01-24 14:48 ` Eli Zaretskii
@ 2016-01-24 14:59   ` Alexander Kuleshov
  2016-01-24 16:07     ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander Kuleshov @ 2016-01-24 14:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 22453, Alexander Kuleshov

Hello, first of all thanks for your feedback.

On 01-24-16, Eli Zaretskii wrote:
> > From: Alexander Kuleshov <kuleshovmail@gmail.com>
> > Date: Sun, 24 Jan 2016 12:30:36 +0600
> > Cc: Alexander Kuleshov <kuleshovmail@gmail.com>
> > 
> > We can easily switch to text mode from the image mode by the
> > pressing of C-c C-c. But sometimes, it is more useful to open
> > an image in hex format. This patch provides new keybinding
> > for the image mode - C-c C-x which works like C-c C-c, but
> > executes switch to the hexl-mode. Like switching to text mode,
> > switching to hex mode supports switching back to the image-mode.
> > 
> > The patch contains following changes:
> > ...
> > ...
> > ...
> > 
> > Patch tested on top of emacs-25 branch on fedora 23 with all
> > Linux related configured options.
> 
> Thanks.  This generally looks good.  A few comments:
> 
>   . This should go to master, not emacs-25, as it's a new feature.

Ah, ok, didn't know will move it into master in next revision.

>   . Please provide a ChangeLog-style commit log message for the
>     changes (some details are in CONTRIBUTE).

Ok, will do, just one question. where do I need add it? As you may
see I added some description to the commit message. Do I need to
do something additional? But anyway, I will look into CONTRIBUTE.

>   . Please provide an update for the user manual, which describes
>     "C-c C-c", to describe "C-x C-x" as well.

Will do.

> Some specific comments below.
> 
> > +(defun image-mode-as-hex ()
> > +  "Set a non-image mode as major mode in combination with image minor mode.
> > +A non-mage major mode found from `auto-mode-alist' or fundamental mode
> > +displays an image file as hex.  `image-minor-mode' provides the key
> > +\\<image-mode-map>\\[image-toggle-hex-display] to switch back  to `image-mode'
> > +to display an image file as the actual image.
> > +
> > +You can use `image-mode-as-text' in `auto-mode-alist' when you wanto
>                                                                   ^^^^^
> A typo.
> 
> > +to display an image file as text initially.
> 
> Why does this sentence talk about text, when the mode is about hex
> display?  Copy/paste mistake?

Yes, anyway it is very similar. Will fix it.

> > +(defun image-toggle-hex-display ()
> > +  "Toggle between image and text display.
> > +If the current buffer is displaying an image file as an image,
> > +call `image-mode-as-hex' to switch to text.  Otherwise, display
> > +the image by calling `image-mode'."
> 
> It is best to avoid referring to another command in a doc string, if
> you can avoid that.  In this case, the reference doesn't add any
> useful information, so I think it should be deleted.

Are you about `image-mode-as-hex`? I've copied it from the image-toggle-display
doc string. Do we need to delete it there too?

> Last, but not least, I think this contribution is too large to accept
> without legal paperwork.  I don't see your copyright assignment on
> file; would you like me to send you the form so you could start the
> paperwork rolling?

Yes, sure.

Thank you.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
  2016-01-24 14:59   ` Alexander Kuleshov
@ 2016-01-24 16:07     ` Eli Zaretskii
  2016-01-24 18:57       ` Alexander Kuleshov
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2016-01-24 16:07 UTC (permalink / raw)
  To: Alexander Kuleshov; +Cc: 22453

> Date: Sun, 24 Jan 2016 20:59:41 +0600
> From: Alexander Kuleshov <kuleshovmail@gmail.com>
> Cc: Alexander Kuleshov <kuleshovmail@gmail.com>, 22453@debbugs.gnu.org
> 
> >   . Please provide a ChangeLog-style commit log message for the
> >     changes (some details are in CONTRIBUTE).
> 
> Ok, will do, just one question. where do I need add it? As you may
> see I added some description to the commit message. Do I need to
> do something additional? But anyway, I will look into CONTRIBUTE.

It is best to make a local commit to your private branch, write the
commit log message using "C-x 4 a", and then produce the patch using
"git format-patch".

> > > +(defun image-toggle-hex-display ()
> > > +  "Toggle between image and text display.
> > > +If the current buffer is displaying an image file as an image,
> > > +call `image-mode-as-hex' to switch to text.  Otherwise, display
> > > +the image by calling `image-mode'."
> > 
> > It is best to avoid referring to another command in a doc string, if
> > you can avoid that.  In this case, the reference doesn't add any
> > useful information, so I think it should be deleted.
> 
> Are you about `image-mode-as-hex`? I've copied it from the image-toggle-display
> doc string. Do we need to delete it there too?

Yes, please delete the reference in both doc strings.

> > Last, but not least, I think this contribution is too large to accept
> > without legal paperwork.  I don't see your copyright assignment on
> > file; would you like me to send you the form so you could start the
> > paperwork rolling?
> 
> Yes, sure.

Sent off-list.

Thanks.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
  2016-01-24 16:07     ` Eli Zaretskii
@ 2016-01-24 18:57       ` Alexander Kuleshov
  2016-01-24 20:33         ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander Kuleshov @ 2016-01-24 18:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 22453, Alexander Kuleshov

I have yet another one question. You wrote:

> Please provide an update for the user manual, which describes
> "C-c C-c", to describe "C-x C-x" as well.

This patch already has description of this. If I select `descover-mode`
I see there:

Image[png] mode defined in ‘image-mode.el’:
Major mode for image files.
You can use C-c C-c or C-c C-x
to toggle between display as an image and display as text or hex.

And also:

C-c C-c         image-toggle-display
C-c C-x         image-toggle-hex-display

Is it enough? I tried to search documentation which is related to
the image-mode in the emacs's source code, but didn't find anything.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
  2016-01-24 18:57       ` Alexander Kuleshov
@ 2016-01-24 20:33         ` Eli Zaretskii
  2016-02-17  7:19           ` Alexander Kuleshov
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2016-01-24 20:33 UTC (permalink / raw)
  To: Alexander Kuleshov; +Cc: 22453

> Date: Mon, 25 Jan 2016 00:57:55 +0600
> From: Alexander Kuleshov <kuleshovmail@gmail.com>
> Cc: Alexander Kuleshov <kuleshovmail@gmail.com>, 22453@debbugs.gnu.org
> 
> I have yet another one question. You wrote:
> 
> > Please provide an update for the user manual, which describes
> > "C-c C-c", to describe "C-x C-x" as well.
> 
> This patch already has description of this. If I select `descover-mode`
> I see there:
> 
> Image[png] mode defined in ‘image-mode.el’:
> Major mode for image files.
> You can use C-c C-c or C-c C-x
> to toggle between display as an image and display as text or hex.
> 
> And also:
> 
> C-c C-c         image-toggle-display
> C-c C-x         image-toggle-hex-display
> 
> Is it enough? I tried to search documentation which is related to
> the image-mode in the emacs's source code, but didn't find anything.

I meant in the user manual, see doc/emacs/files.texi and search for
"image-mode" inside that file.

Thanks.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
  2016-01-24 20:33         ` Eli Zaretskii
@ 2016-02-17  7:19           ` Alexander Kuleshov
  2016-02-20 12:30             ` Eli Zaretskii
  2016-02-27 15:15             ` Alexander Kuleshov
  0 siblings, 2 replies; 10+ messages in thread
From: Alexander Kuleshov @ 2016-02-17  7:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 22453

From bc003ed7409a9b4c3bfb1444904c223775f3d18e Mon Sep 17 00:00:00 2001
From: Alexander Kuleshov <kuleshovmail@gmail.com>
Date: Sat, 23 Jan 2016 21:13:37 +0600
Subject: [PATCH] Support for switching to hexl-mode from image mode

* lisp/image-mode.el: adapted comments at the top of file;
* lisp/image-mode.el: added new menu item to the Image menu:
'Show as hex';
* lisp/image-mode.el: added description of the new keybinding
to the image-mode help;
* lisp/image-mode.el: all help messages in minibuffer adapted
to new keybinding;
* lisp/image-mode.el: image-toggle-help display function added
which checks current mode and switch into text or image mode
depends on it;
* doc/emacs/files.texi: added description of C-c C-x keybinding.

Patch tested on top of emacs-25 branch on fedora 23 with all
Linux related configured options.
---
 doc/emacs/files.texi |  3 ++
 lisp/image-mode.el   | 98 ++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 72 insertions(+), 29 deletions(-)

diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index ab20d4a..bbb6070 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -1969,6 +1969,9 @@ File Conveniences
 major mode, you can type @kbd{C-c C-c} (@code{image-toggle-display})
 to toggle between displaying the file as an image in the Emacs buffer,
 and displaying its underlying text (or raw byte) representation.
+Additionally you can type @kbd{C-c C-x} (@code{image-toggle-hex-display})
+to toggle between displaying the file as an image in the Emacs buffer,
+and displaying it in hex representation.
 Displaying the file as an image works only if Emacs is compiled with
 support for displaying such images.  If the displayed image is wider
 or taller than the frame, the usual point motion keys (@kbd{C-f},
diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index e549b49..a3cc3db 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -24,8 +24,8 @@
 ;;; Commentary:

 ;; Defines a major mode for visiting image files
-;; that allows conversion between viewing the text of the file
-;; and viewing the file as an image.  Viewing the image
+;; that allows conversion between viewing the text of the file,
+;; hex of the file and viewing the file as an image.  Viewing the image
 ;; works by putting a `display' text-property on the
 ;; image data, with the image-data still present underneath; if the
 ;; resulting buffer file is saved to another name it will correctly save
@@ -374,6 +374,7 @@ image-mode-map
   (let ((map (make-sparse-keymap)))
     (set-keymap-parent map special-mode-map)
     (define-key map "\C-c\C-c" 'image-toggle-display)
+    (define-key map "\C-c\C-x" 'image-toggle-hex-display)
     (define-key map (kbd "SPC")       'image-scroll-up)
     (define-key map (kbd "S-SPC")     'image-scroll-down)
     (define-key map (kbd "DEL")       'image-scroll-down)
@@ -406,6 +407,8 @@ image-mode-map
       '("Image"
     ["Show as Text" image-toggle-display :active t
      :help "Show image as text"]
+    ["Show as Hex" image-toggle-hex-display :active t
+     :help "Show image as hex"]
     "--"
     ["Fit to Window Height" image-transform-fit-to-height
      :visible (eq image-type 'imagemagick)
@@ -480,6 +483,7 @@ image-mode-map
 (defvar image-minor-mode-map
   (let ((map (make-sparse-keymap)))
     (define-key map "\C-c\C-c" 'image-toggle-display)
+    (define-key map "\C-c\C-x" 'image-toggle-hex-display)
     map)
   "Mode keymap for `image-minor-mode'.")

@@ -490,8 +494,8 @@ bookmark-make-record-function
 ;;;###autoload
 (defun image-mode ()
   "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.
+You can use \\<image-mode-map>\\[image-toggle-display] or
\\<image-mode-map>\\[image-toggle-hex-display]
+to toggle between display as an image and display as text or hex.

 Key bindings:
 \\{image-mode-map}"
@@ -530,7 +534,7 @@ image-mode
     (run-mode-hooks 'image-mode-hook)
     (let ((image (image-get-display-property))
           (msg1 (substitute-command-keys
-             "Type \\[image-toggle-display] to view the image as "))
+             "Type \\[image-toggle-display] or
\\[image-toggle-hex-display] to view the image as "))
           animated)
       (cond
        ((null image)
@@ -559,7 +563,7 @@ image-mode
 ;;;                 (substitute-command-keys
 ;;;                  "\\[image-toggle-animation] to animate."))))
        (t
-        (message "%s" (concat msg1 "text."))))))
+        (message "%s" (concat msg1 "text or hex."))))))

     (error
      (image-mode-as-text)
@@ -585,19 +589,10 @@ image-minor-mode
       (add-hook 'change-major-mode-hook (lambda () (image-minor-mode
-1)) nil t)))

 ;;;###autoload
-(defun image-mode-as-text ()
+(defun image-mode-to-text ()
   "Set a non-image mode as major mode in combination with image minor mode.
-A non-image major mode found from `auto-mode-alist' or Fundamental mode
-displays an image file as text.  `image-minor-mode' provides the key
-\\<image-mode-map>\\[image-toggle-display] to switch back to `image-mode'
-to display an image file as the actual image.
-
-You can use `image-mode-as-text' in `auto-mode-alist' when you want
-to display an image file as text initially.
-
-See commands `image-mode' and `image-minor-mode' for more information
-on these modes."
-  (interactive)
+A non-mage major mode found from `auto-mode-alist' or fundamental mode
+displays an image file as text."
   ;; image-mode-as-text = normal-mode + image-minor-mode
   (let ((previous-image-type image-type)) ; preserve `image-type'
     (if image-mode-previous-major-mode
@@ -625,12 +620,49 @@ image-mode-as-text
     ;; Enable image minor mode with `C-c C-c'.
     (image-minor-mode 1)
     ;; Show the image file as text.
-    (image-toggle-display-text)
-    (message "%s" (concat
-           (substitute-command-keys
-            "Type \\[image-toggle-display] to view the image as ")
-           (if (image-get-display-property)
-               "text" "an image") "."))))
+    (image-toggle-display-text)))
+
+(defun image-mode-as-hex ()
+  "Set a non-image mode as major mode in combination with image minor mode.
+A non-mage major mode found from `auto-mode-alist' or fundamental mode
+displays an image file as hex.  `image-minor-mode' provides the key
+\\<image-mode-map>\\[image-toggle-hex-display] to switch back to `image-mode'
+to display an image file as the actual image.
+
+You can use `image-mode-as-hex' in `auto-mode-alist' when you want to
+to display an image file as hex initially.
+
+See commands `image-mode' and `image-minor-mode' for more information
+on these modes."
+  (interactive)
+  (image-mode-to-text)
+  ;; Turn on hexl-mode
+  (hexl-mode)
+  (message "%s" (concat
+                 (substitute-command-keys
+                  "Type \\[image-toggle-hex-display] or
\\[image-toggle-display] to view the image as ")
+                 (if (image-get-display-property)
+                     "hex" "an image or text") ".")))
+
+(defun image-mode-as-text ()
+  "Set a non-image mode as major mode in combination with image minor mode.
+A non-image major mode found from `auto-mode-alist' or Fundamental mode
+displays an image file as text.  `image-minor-mode' provides the key
+\\<image-mode-map>\\[image-toggle-display] to switch back to `image-mode'
+to display an image file as the actual image.
+
+You can use `image-mode-as-text' in `auto-mode-alist' when you want
+to display an image file as text initially.
+
+See commands `image-mode' and `image-minor-mode' for more information
+on these modes."
+  (interactive)
+  (image-mode-to-text)
+  (message "%s" (concat
+                 (substitute-command-keys
+                  "Type \\[image-toggle-display] or
\\[image-toggle-hex-display] to view the image as ")
+                 (if (image-get-display-property)
+                     "text" "an image or hex") ".")))

 (define-obsolete-function-alias 'image-mode-maybe 'image-mode "23.2")

@@ -725,15 +757,23 @@ image-toggle-display-image
     (if (called-interactively-p 'any)
     (message "Repeat this command to go back to displaying the file
as text"))))

+(defun image-toggle-hex-display ()
+  "Toggle between image and hex display."
+  (interactive)
+  (if (image-get-display-property)
+      (image-mode-as-hex)
+    (if (eq major-mode 'fundamental-mode)
+        (image-mode-as-hex)
+      (image-mode))))
+
 (defun image-toggle-display ()
-  "Toggle between image and text display.
-If the current buffer is displaying an image file as an image,
-call `image-mode-as-text' to switch to text.  Otherwise, display
-the image by calling `image-mode'."
+  "Toggle between image and text display."
   (interactive)
   (if (image-get-display-property)
       (image-mode-as-text)
-    (image-mode)))
+    (if (eq major-mode 'hexl-mode)
+        (image-mode-as-text)
+      (image-mode))))

 (defun image-kill-buffer ()
   "Kill the current buffer."
-- 
2.7.0.364.g4943984





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
  2016-02-17  7:19           ` Alexander Kuleshov
@ 2016-02-20 12:30             ` Eli Zaretskii
  2016-02-27 15:15             ` Alexander Kuleshov
  1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2016-02-20 12:30 UTC (permalink / raw)
  To: Alexander Kuleshov; +Cc: 22453

> From: Alexander Kuleshov <kuleshovmail@gmail.com>
> Date: Wed, 17 Feb 2016 13:19:24 +0600
> Cc: 22453@debbugs.gnu.org
> 
> >From bc003ed7409a9b4c3bfb1444904c223775f3d18e Mon Sep 17 00:00:00 2001
> From: Alexander Kuleshov <kuleshovmail@gmail.com>
> Date: Sat, 23 Jan 2016 21:13:37 +0600
> Subject: [PATCH] Support for switching to hexl-mode from image mode
> 
> * lisp/image-mode.el: adapted comments at the top of file;
> * lisp/image-mode.el: added new menu item to the Image menu:
> 'Show as hex';
> * lisp/image-mode.el: added description of the new keybinding
> to the image-mode help;
> * lisp/image-mode.el: all help messages in minibuffer adapted
> to new keybinding;
> * lisp/image-mode.el: image-toggle-help display function added
> which checks current mode and switch into text or image mode
> depends on it;
> * doc/emacs/files.texi: added description of C-c C-x keybinding.
> 
> Patch tested on top of emacs-25 branch on fedora 23 with all
> Linux related configured options.

Thanks.  This is still relative to emacs-25; it should be relative to
master.  Also, the log entries above are not according to the
conventions described in CONTRIBUTE.

When you fix the above 2 gotchas, we will need to wait until you legal
paperwork comes through: I don't see your assignment on file, as of
now.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
  2016-02-17  7:19           ` Alexander Kuleshov
  2016-02-20 12:30             ` Eli Zaretskii
@ 2016-02-27 15:15             ` Alexander Kuleshov
  2016-02-27 17:31               ` Eli Zaretskii
  1 sibling, 1 reply; 10+ messages in thread
From: Alexander Kuleshov @ 2016-02-27 15:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 22453

Add support for switching to hexl-mode from image mode

* doc/emacs/files.texi: Document image-toggle-hex-display

* lisp/image-mode.el (image-toggle-hex-display, image-mode-to-text):
New functions
---
 doc/emacs/files.texi |  3 ++
 lisp/image-mode.el   | 98 ++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 72 insertions(+), 29 deletions(-)

diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index ab20d4a..bbb6070 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -1969,6 +1969,9 @@ File Conveniences
 major mode, you can type @kbd{C-c C-c} (@code{image-toggle-display})
 to toggle between displaying the file as an image in the Emacs buffer,
 and displaying its underlying text (or raw byte) representation.
+Additionally you can type @kbd{C-c C-x} (@code{image-toggle-hex-display})
+to toggle between displaying the file as an image in the Emacs buffer,
+and displaying it in hex representation.
 Displaying the file as an image works only if Emacs is compiled with
 support for displaying such images.  If the displayed image is wider
 or taller than the frame, the usual point motion keys (@kbd{C-f},
diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index e2037b9..2d560d3 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -24,8 +24,8 @@
 ;;; Commentary:

 ;; Defines a major mode for visiting image files
-;; that allows conversion between viewing the text of the file
-;; and viewing the file as an image.  Viewing the image
+;; that allows conversion between viewing the text of the file,
+;; hex of the file and viewing the file as an image.  Viewing the image
 ;; works by putting a `display' text-property on the
 ;; image data, with the image-data still present underneath; if the
 ;; resulting buffer file is saved to another name it will correctly save
@@ -375,6 +375,7 @@ image-mode-map
     (set-keymap-parent map special-mode-map)
     (set-keymap-parent map image-map)
     (define-key map "\C-c\C-c" 'image-toggle-display)
+    (define-key map "\C-c\C-x" 'image-toggle-hex-display)
     (define-key map (kbd "SPC")       'image-scroll-up)
     (define-key map (kbd "S-SPC")     'image-scroll-down)
     (define-key map (kbd "DEL")       'image-scroll-down)
@@ -407,6 +408,8 @@ image-mode-map
       '("Image"
     ["Show as Text" image-toggle-display :active t
      :help "Show image as text"]
+    ["Show as Hex" image-toggle-hex-display :active t
+     :help "Show image as hex"]
     "--"
     ["Fit to Window Height" image-transform-fit-to-height
      :visible (eq image-type 'imagemagick)
@@ -481,6 +484,7 @@ image-mode-map
 (defvar image-minor-mode-map
   (let ((map (make-sparse-keymap)))
     (define-key map "\C-c\C-c" 'image-toggle-display)
+    (define-key map "\C-c\C-x" 'image-toggle-hex-display)
     map)
   "Mode keymap for `image-minor-mode'.")

@@ -491,8 +495,8 @@ bookmark-make-record-function
 ;;;###autoload
 (defun image-mode ()
   "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.
+You can use \\<image-mode-map>\\[image-toggle-display] or
\\<image-mode-map>\\[image-toggle-hex-display]
+to toggle between display as an image and display as text or hex.

 Key bindings:
 \\{image-mode-map}"
@@ -531,7 +535,7 @@ image-mode
     (run-mode-hooks 'image-mode-hook)
     (let ((image (image-get-display-property))
           (msg1 (substitute-command-keys
-             "Type \\[image-toggle-display] to view the image as "))
+             "Type \\[image-toggle-display] or
\\[image-toggle-hex-display] to view the image as "))
           animated)
       (cond
        ((null image)
@@ -560,7 +564,7 @@ image-mode
 ;;;                 (substitute-command-keys
 ;;;                  "\\[image-toggle-animation] to animate."))))
        (t
-        (message "%s" (concat msg1 "text."))))))
+        (message "%s" (concat msg1 "text or hex."))))))

     (error
      (image-mode-as-text)
@@ -586,19 +590,10 @@ image-minor-mode
       (add-hook 'change-major-mode-hook (lambda () (image-minor-mode
-1)) nil t)))

 ;;;###autoload
-(defun image-mode-as-text ()
+(defun image-mode-to-text ()
   "Set a non-image mode as major mode in combination with image minor mode.
-A non-image major mode found from `auto-mode-alist' or Fundamental mode
-displays an image file as text.  `image-minor-mode' provides the key
-\\<image-mode-map>\\[image-toggle-display] to switch back to `image-mode'
-to display an image file as the actual image.
-
-You can use `image-mode-as-text' in `auto-mode-alist' when you want
-to display an image file as text initially.
-
-See commands `image-mode' and `image-minor-mode' for more information
-on these modes."
-  (interactive)
+A non-mage major mode found from `auto-mode-alist' or fundamental mode
+displays an image file as text."
   ;; image-mode-as-text = normal-mode + image-minor-mode
   (let ((previous-image-type image-type)) ; preserve `image-type'
     (if image-mode-previous-major-mode
@@ -626,12 +621,49 @@ image-mode-as-text
     ;; Enable image minor mode with `C-c C-c'.
     (image-minor-mode 1)
     ;; Show the image file as text.
-    (image-toggle-display-text)
-    (message "%s" (concat
-           (substitute-command-keys
-            "Type \\[image-toggle-display] to view the image as ")
-           (if (image-get-display-property)
-               "text" "an image") "."))))
+    (image-toggle-display-text)))
+
+(defun image-mode-as-hex ()
+  "Set a non-image mode as major mode in combination with image minor mode.
+A non-mage major mode found from `auto-mode-alist' or fundamental mode
+displays an image file as hex.  `image-minor-mode' provides the key
+\\<image-mode-map>\\[image-toggle-hex-display] to switch back to `image-mode'
+to display an image file as the actual image.
+
+You can use `image-mode-as-hex' in `auto-mode-alist' when you want to
+to display an image file as hex initially.
+
+See commands `image-mode' and `image-minor-mode' for more information
+on these modes."
+  (interactive)
+  (image-mode-to-text)
+  ;; Turn on hexl-mode
+  (hexl-mode)
+  (message "%s" (concat
+                 (substitute-command-keys
+                  "Type \\[image-toggle-hex-display] or
\\[image-toggle-display] to view the image as ")
+                 (if (image-get-display-property)
+                     "hex" "an image or text") ".")))
+
+(defun image-mode-as-text ()
+  "Set a non-image mode as major mode in combination with image minor mode.
+A non-image major mode found from `auto-mode-alist' or Fundamental mode
+displays an image file as text.  `image-minor-mode' provides the key
+\\<image-mode-map>\\[image-toggle-display] to switch back to `image-mode'
+to display an image file as the actual image.
+
+You can use `image-mode-as-text' in `auto-mode-alist' when you want
+to display an image file as text initially.
+
+See commands `image-mode' and `image-minor-mode' for more information
+on these modes."
+  (interactive)
+  (image-mode-to-text)
+  (message "%s" (concat
+                 (substitute-command-keys
+                  "Type \\[image-toggle-display] or
\\[image-toggle-hex-display] to view the image as ")
+                 (if (image-get-display-property)
+                     "text" "an image or hex") ".")))

 (define-obsolete-function-alias 'image-mode-maybe 'image-mode "23.2")

@@ -726,16 +758,23 @@ image-toggle-display-image
     (if (called-interactively-p 'any)
     (message "Repeat this command to go back to displaying the file
as text"))))

+(defun image-toggle-hex-display ()
+  "Toggle between image and hex display."
+  (interactive)
+  (if (image-get-display-property)
+      (image-mode-as-hex)
+    (if (eq major-mode 'fundamental-mode)
+        (image-mode-as-hex)
+      (image-mode))))
+
 (defun image-toggle-display ()
-  "Toggle between image and text display.
-If the current buffer is displaying an image file as an image,
-call `image-mode-as-text' to switch to text.  Otherwise, display
-the image by calling `image-mode'."
+  "Toggle between image and text display."
   (interactive)
   (if (image-get-display-property)
       (image-mode-as-text)
-    (image-mode)))
+    (if (eq major-mode 'hexl-mode)
+        (image-mode-as-text)
+      (image-mode))))

 (defun image-kill-buffer ()
   "Kill the current buffer."
--
2.7.2.335.g3476f70





^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#22453: [PATCH] Support for switching to hexl-mode from image mode
  2016-02-27 15:15             ` Alexander Kuleshov
@ 2016-02-27 17:31               ` Eli Zaretskii
  0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2016-02-27 17:31 UTC (permalink / raw)
  To: Alexander Kuleshov; +Cc: 22453-done

> From: Alexander Kuleshov <kuleshovmail@gmail.com>
> Date: Sat, 27 Feb 2016 21:15:44 +0600
> Cc: 22453@debbugs.gnu.org
> 
> Add support for switching to hexl-mode from image mode
> 
> * doc/emacs/files.texi: Document image-toggle-hex-display
> 
> * lisp/image-mode.el (image-toggle-hex-display, image-mode-to-text):
> New functions
> ---
>  doc/emacs/files.texi |  3 ++
>  lisp/image-mode.el   | 98 ++++++++++++++++++++++++++++++++++++----------------
>  2 files changed, 72 insertions(+), 29 deletions(-)

Thanks, pushed to master.

Btw, your email client wraps lines, so the patch didn't apply, and I
had to do it manually.  Perhaps you should send patches as
attachments.

Also, please see how I formatted the log message, and do the same in
the future.  In particular, quite a few of your changes were never
mentioned in your log message.





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-02-27 17:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-24  6:30 bug#22453: [PATCH] Support for switching to hexl-mode from image mode Alexander Kuleshov
2016-01-24 14:48 ` Eli Zaretskii
2016-01-24 14:59   ` Alexander Kuleshov
2016-01-24 16:07     ` Eli Zaretskii
2016-01-24 18:57       ` Alexander Kuleshov
2016-01-24 20:33         ` Eli Zaretskii
2016-02-17  7:19           ` Alexander Kuleshov
2016-02-20 12:30             ` Eli Zaretskii
2016-02-27 15:15             ` Alexander Kuleshov
2016-02-27 17:31               ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).