bug#22453: [PATCH] Support for switching to hexl-mode from image mode

[Eli Zaretskii]

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