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

[Alexander Kuleshov <kuleshovmail@gmail.com>]

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.