unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Image-conversion shims
@ 2019-09-28 20:06 Lars Ingebrigtsen
  2019-09-28 20:37 ` Clément Pit-Claudel
  2019-09-28 21:05 ` Stefan Monnier
  0 siblings, 2 replies; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-28 20:06 UTC (permalink / raw)
  To: emacs-devel

I suggested this in an unrelated bug report, but nobody probably saw it
there, so I'm bringing it up here again:

Emacs has approximate feature parity for the major image formats (JPEG,
PNG, etc) now with ImageMagick-enabled Emacs versions.  I think our
long-term plan is to obsolete the ImageMagick support completely,
because ImageMagick has some security issues.

However, the only way to support more obscure image formats (like BMP or
WebP) is currently to build with ImageMagick support, so it seems likely
to me that distributions are going to continue doing that.

So I wonder whether a way to speed up the transition to an
ImageMagick-less future would be to have a small package that would
convert "transparently" from formats we don't understand to formats we
do understand.

The only likely external conversion program is, unfortunately, "convert"
from ImageMagick, so there are still security implications, and a shim
like that isn't something we could have switched on by default.
However, we would be able to say to users who need to handle some BMP
images "set `use-external-image-formats' to t" instead of "build with
ImageMagick", and that would be a win, I think.  (Because Emacs would at
least segfault less.)

If that variable is t, and we don't have support for whatever image
format the user is trying to display, then `create-image' would use the
conversion package to basically run "convert" on the file first.  And
"convert -list format/magic" tell us what formats it supports, so it
could all be made to work automagically.

Thoughts?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




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

* Re: Image-conversion shims
  2019-09-28 20:06 Image-conversion shims Lars Ingebrigtsen
@ 2019-09-28 20:37 ` Clément Pit-Claudel
  2019-09-28 21:20   ` Lars Ingebrigtsen
  2019-09-29  6:24   ` Eli Zaretskii
  2019-09-28 21:05 ` Stefan Monnier
  1 sibling, 2 replies; 30+ messages in thread
From: Clément Pit-Claudel @ 2019-09-28 20:37 UTC (permalink / raw)
  To: emacs-devel

On 2019-09-28 16:06, Lars Ingebrigtsen wrote:
> The only likely external conversion program is, unfortunately, "convert"
> from ImageMagick, so there are still security implications, and a shim
> like that isn't something we could have switched on by default.

An alternative route that might be worth exploring is http://www.graphicsmagick.org/, which is GPL-compatible and seems to have a better security record than ImageMagick.

Clément.



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

* Re: Image-conversion shims
  2019-09-28 20:06 Image-conversion shims Lars Ingebrigtsen
  2019-09-28 20:37 ` Clément Pit-Claudel
@ 2019-09-28 21:05 ` Stefan Monnier
  2019-09-28 21:21   ` Lars Ingebrigtsen
  2019-09-28 23:29   ` Lars Ingebrigtsen
  1 sibling, 2 replies; 30+ messages in thread
From: Stefan Monnier @ 2019-09-28 21:05 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: emacs-devel

> I think our long-term plan is to obsolete the ImageMagick support
> completely, because ImageMagick has some security issues.

Indeed (security issues and a lack of maintenance, AFAIK).

> So I wonder whether a way to speed up the transition to an
> ImageMagick-less future would be to have a small package that would
> convert "transparently" from formats we don't understand to formats we
> do understand.

Sounds good to me.

> The only likely external conversion program is, unfortunately, "convert"
> from ImageMagick,

Really?  ffmpeg can also convert from WebP to PNG, and I suspect there
are many other options for BMP.

> Thoughts?

Go for it.  BTW, I just saw that FLIF is a (usually) better alternative
to WebP, so maybe we should add native support for that ;-)


        Stefan




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

* Re: Image-conversion shims
  2019-09-28 20:37 ` Clément Pit-Claudel
@ 2019-09-28 21:20   ` Lars Ingebrigtsen
  2019-09-29  6:24   ` Eli Zaretskii
  1 sibling, 0 replies; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-28 21:20 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: emacs-devel

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:

> On 2019-09-28 16:06, Lars Ingebrigtsen wrote:
>> The only likely external conversion program is, unfortunately, "convert"
>> from ImageMagick, so there are still security implications, and a shim
>> like that isn't something we could have switched on by default.
>
> An alternative route that might be worth exploring is
> http://www.graphicsmagick.org/, which is GPL-compatible and seems to
> have a better security record than ImageMagick.

Ah, I didn't know that it had a command line interface, too.  "gm
convert" seems like is should be almost drop-in compatible with
"convert".  This shim library could support a number of command line
functions, I guess, depending on what's installed
(gm/convert/ffmpeg/netpbm).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-09-28 21:05 ` Stefan Monnier
@ 2019-09-28 21:21   ` Lars Ingebrigtsen
  2019-09-28 23:29   ` Lars Ingebrigtsen
  1 sibling, 0 replies; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-28 21:21 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

>> The only likely external conversion program is, unfortunately, "convert"
>> from ImageMagick,
>
> Really?  ffmpeg can also convert from WebP to PNG, and I suspect there
> are many other options for BMP.

Indeed.  I've never used ffmpeg for anything but video conversion, but
"ffmpeg -decoders" lists pretty much everything (including BMP and
WebP).

> Go for it.  BTW, I just saw that FLIF is a (usually) better alternative
> to WebP, so maybe we should add native support for that ;-)

:-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-09-28 21:05 ` Stefan Monnier
  2019-09-28 21:21   ` Lars Ingebrigtsen
@ 2019-09-28 23:29   ` Lars Ingebrigtsen
  2019-09-29  0:01     ` Lars Ingebrigtsen
  2019-09-29  7:11     ` Eli Zaretskii
  1 sibling, 2 replies; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-28 23:29 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

> Go for it. 

I went for it.

(setq convert-images-externally t)
(insert-image (create-image "/var/tmp/1sm.webp"))

now works for me.

It doesn't work in image-mode yet, because I was unsure at what point to
hook the auto-mode stuff together with this stuff.  It's slightly
chicken and egg -- convert-images-externally isn't consulted before we
try to display an image, and it will then compute what image suffixes it
can handle.

But if that hasn't been computed, then image-mode won't be triggered,
and so create-image won't be called.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-09-28 23:29   ` Lars Ingebrigtsen
@ 2019-09-29  0:01     ` Lars Ingebrigtsen
  2019-09-29  7:11     ` Eli Zaretskii
  1 sibling, 0 replies; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-29  0:01 UTC (permalink / raw)
  To: emacs-devel

Lars Ingebrigtsen <larsi@gnus.org> writes:

> (setq convert-images-externally t)
> (insert-image (create-image "/var/tmp/1sm.webp"))

In case anybody wants to play with this and need a sample webp image:

curl -O https://www.gstatic.com/webp/gallery/1.sm.webp

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-09-28 20:37 ` Clément Pit-Claudel
  2019-09-28 21:20   ` Lars Ingebrigtsen
@ 2019-09-29  6:24   ` Eli Zaretskii
  1 sibling, 0 replies; 30+ messages in thread
From: Eli Zaretskii @ 2019-09-29  6:24 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: emacs-devel

> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Sat, 28 Sep 2019 16:37:15 -0400
> 
> An alternative route that might be worth exploring is http://www.graphicsmagick.org/, which is GPL-compatible and seems to have a better security record than ImageMagick.

The better record of GraphicsMagick notwithstanding, I think going
that way would be a step back, as every additional dependency library
of such complexity adds a significant amount of hair in the Emacs code
and maintenance burden.

We could, of course, use the GraphicsMagick's equivalent of 'convert',
as another alternative of an external converter, if we decide to add
the feature suggested by Lars.



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

* Re: Image-conversion shims
  2019-09-28 23:29   ` Lars Ingebrigtsen
  2019-09-29  0:01     ` Lars Ingebrigtsen
@ 2019-09-29  7:11     ` Eli Zaretskii
  2019-09-29 11:44       ` Lars Ingebrigtsen
  1 sibling, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2019-09-29  7:11 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: monnier, emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sun, 29 Sep 2019 01:29:48 +0200
> Cc: emacs-devel@gnu.org
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > Go for it. 
> 
> I went for it.

Thanks.

> (setq convert-images-externally t)

That variable's name should start with "image-", IMO, and the same for
the (confusingly similar) convert-external-images.  It should also be
documented in the user manual (I'd simply move what you wrote for the
ELisp manual to the user manual, since I see no reason to document
this in the former).

When looking for an installed converter, shouldn't we put ImageMagick
last?

I also think that using call-process for invoking the converter makes
the feature less flexible and more "tricky" to maintain.  Already you
needed to jump through some hoops to support "gm convert".  I think
using shell-command would have made all this much simpler and more
straightforward.

> It doesn't work in image-mode yet, because I was unsure at what point to
> hook the auto-mode stuff together with this stuff.  It's slightly
> chicken and egg -- convert-images-externally isn't consulted before we
> try to display an image, and it will then compute what image suffixes it
> can handle.
> 
> But if that hasn't been computed, then image-mode won't be triggered,
> and so create-image won't be called.

Maybe we need a new hook?



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

* Re: Image-conversion shims
  2019-09-29  7:11     ` Eli Zaretskii
@ 2019-09-29 11:44       ` Lars Ingebrigtsen
  2019-09-29 11:56         ` Eli Zaretskii
  0 siblings, 1 reply; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-29 11:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> (setq convert-images-externally t)
>
> That variable's name should start with "image-", IMO, and the same for
> the (confusingly similar) convert-external-images.

Hm...  I moved that variable from image-converter.el to image.el and
slightly renamed it, but apparently forgot to remove it.  Now fixed.

I tried to come up with several variations for the variable name, but
they all sounded even more awkward.

Uhm...  what about...  image-use-external-converter?

> It should also be documented in the user manual (I'd simply move what
> you wrote for the ELisp manual to the user manual, since I see no
> reason to document this in the former).

Makes sense; now done.

> When looking for an installed converter, shouldn't we put ImageMagick
> last?

Yup.

> I also think that using call-process for invoking the converter makes
> the feature less flexible and more "tricky" to maintain.  Already you
> needed to jump through some hoops to support "gm convert".  I think
> using shell-command would have made all this much simpler and more
> straightforward.

It's true that it's simpler, but I've been bitten by corner cases in
escape handling of file names before (and we're dealing with file names
that are under some degree of control by an attacker), so I just think
it's easier to not have to think about those issues at all (i.e., use
call-process).

>> But if that hasn't been computed, then image-mode won't be triggered,
>> and so create-image won't be called.
>
> Maybe we need a new hook?

That would be possible, but would require more work for the user,
possibly.

I was pondering whether something could be done in `set-auto-mode'.  It
already consults a wide number of variables to determine the mode.
Could we add yet another one that's more flexible?

That is, it would be an alist on the form (variable . action) and would
be consulted last in that function as the final fallback.  Like:

(defvar auto-mode-dependent-action-alist
  '((convert-images-externally . image-image-converter-file-alist)))

and the code would be

(dolist (elem auto-mode-dependent-action-alist)
  (when (bound-and-true-p (car elem))
    (let ((alist (cdr elem)))
      ;; Do the same thing as with auto-mode-alist
      ))

It would provide a mechanism for users to switch on/off a group of mode
alist entries with a single setq...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-09-29 11:44       ` Lars Ingebrigtsen
@ 2019-09-29 11:56         ` Eli Zaretskii
  2019-09-30  4:12           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2019-09-29 11:56 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: monnier, emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Sun, 29 Sep 2019 13:44:41 +0200
> 
> Uhm...  what about...  image-use-external-converter?

SGTM.

> > I also think that using call-process for invoking the converter makes
> > the feature less flexible and more "tricky" to maintain.  Already you
> > needed to jump through some hoops to support "gm convert".  I think
> > using shell-command would have made all this much simpler and more
> > straightforward.
> 
> It's true that it's simpler, but I've been bitten by corner cases in
> escape handling of file names before (and we're dealing with file names
> that are under some degree of control by an attacker), so I just think
> it's easier to not have to think about those issues at all (i.e., use
> call-process).

But that means the command strings are restricted in what they can
use, because split-string has some limitations that aren't immediately
evident.  So if you want to leave this stuff as-is, at least we should
say something in the doc string of image-converter--converters to that
effect that; e.g., I think quotes should not be allowed there.

> I was pondering whether something could be done in `set-auto-mode'.  It
> already consults a wide number of variables to determine the mode.
> Could we add yet another one that's more flexible?
> 
> That is, it would be an alist on the form (variable . action) and would
> be consulted last in that function as the final fallback.  Like:
> 
> (defvar auto-mode-dependent-action-alist
>   '((convert-images-externally . image-image-converter-file-alist)))
> 
> and the code would be
> 
> (dolist (elem auto-mode-dependent-action-alist)
>   (when (bound-and-true-p (car elem))
>     (let ((alist (cdr elem)))
>       ;; Do the same thing as with auto-mode-alist
>       ))

Maybe.  It sounds a bit too convoluted to me, and I hoped for a
simpler solution...

Actually, why not simply add the relevant extensions to
auto-mode-alist, causing them to invoke image-mode?



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

* Re: Image-conversion shims
  2019-09-29 11:56         ` Eli Zaretskii
@ 2019-09-30  4:12           ` Lars Ingebrigtsen
  2019-09-30  7:06             ` Eli Zaretskii
                               ` (2 more replies)
  0 siblings, 3 replies; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-30  4:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Uhm...  what about...  image-use-external-converter?
>
> SGTM.

OK; done.

> But that means the command strings are restricted in what they can
> use, because split-string has some limitations that aren't immediately
> evident.  So if you want to leave this stuff as-is, at least we should
> say something in the doc string of image-converter--converters to that
> effect that; e.g., I think quotes should not be allowed there.

Stefan M. suggested using a list instead of a string, and I think I'll
make that adjustment.

> Maybe.  It sounds a bit too convoluted to me, and I hoped for a
> simpler solution...

Me too, but I don't see one at the moment.

> Actually, why not simply add the relevant extensions to
> auto-mode-alist, causing them to invoke image-mode?

But when do we do that?  To determine what the list of supported formats
is, we have to run `image-converter--probe' (etc).  But we don't know
whether we need to do that until we know that we're going to load an
unsupported image format.  :-/

I.e., we want ("*.webp" . image-mode) in auto-mode-alist, but we can't
put it there before we've called "gm convert -list format", and we don't
do that until we call `create-image' with image-use-external-converter
non-nil, which won't be called since we don't have a mapping from
"*.webp" to image-mode.

Very chicken and egg.

Hm...  One possibility would be to do away with

(setq image-use-external-converter t)

as a user-level thing, and instead tell them to do

(image-external-converter-mode)

in ~/.emacs, which would do the probing and then altering
auto-mode-alist.  Hm.  Perhaps that's a pretty clean idea?  Hm...  I
think so.  Except that switching the mode off would have to remove the
entries from auto-mode-alist again, which sounds slightly destructive.

Uhm.  Perhaps map .wepb etc to a new image-mode-external that should
just be a mode that inherits from image-mode, but would do nothing if
the image-external-converter-mode has been toggled off?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-09-30  4:12           ` Lars Ingebrigtsen
@ 2019-09-30  7:06             ` Eli Zaretskii
  2019-09-30 13:42               ` Lars Ingebrigtsen
  2019-09-30 17:27             ` Stefan Monnier
  2019-10-06 20:58             ` Juri Linkov
  2 siblings, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2019-09-30  7:06 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: monnier, emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Mon, 30 Sep 2019 06:12:03 +0200
> 
> > Actually, why not simply add the relevant extensions to
> > auto-mode-alist, causing them to invoke image-mode?
> 
> But when do we do that?  To determine what the list of supported formats
> is, we have to run `image-converter--probe' (etc).  But we don't know
> whether we need to do that until we know that we're going to load an
> unsupported image format.  :-/
> 
> I.e., we want ("*.webp" . image-mode) in auto-mode-alist, but we can't
> put it there before we've called "gm convert -list format"

I don't think I understand why we can't put the association in
auto-mode-alist before running "gm convert".  Please elaborate why.

> Uhm.  Perhaps map .wepb etc to a new image-mode-external that should
> just be a mode that inherits from image-mode, but would do nothing if
> the image-external-converter-mode has been toggled off?

Not "do nothing", but display an error message saying this image
format is unsupported, IMO.



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

* Re: Image-conversion shims
  2019-09-30  7:06             ` Eli Zaretskii
@ 2019-09-30 13:42               ` Lars Ingebrigtsen
  2019-09-30 14:01                 ` Eli Zaretskii
  0 siblings, 1 reply; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-30 13:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> I don't think I understand why we can't put the association in
> auto-mode-alist before running "gm convert".  Please elaborate why.

If we put (say) .webp into auto-mode-alist, but we can't read .webp
files, then image-mode will end up issuing an error when called on these
files.

Besides, what formats "gm convert" supports changes over time.  By
consulting it, auto-mode-alist will be automatically updated without us
having to do anything.

>> Uhm.  Perhaps map .wepb etc to a new image-mode-external that should
>> just be a mode that inherits from image-mode, but would do nothing if
>> the image-external-converter-mode has been toggled off?
>
> Not "do nothing", but display an error message saying this image
> format is unsupported, IMO.

Yup.  I meant "do nothing" in the image-mode context, which means "show
the data as raw bytes and signal an error".

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-09-30 13:42               ` Lars Ingebrigtsen
@ 2019-09-30 14:01                 ` Eli Zaretskii
  2019-10-01 12:06                   ` Lars Ingebrigtsen
  0 siblings, 1 reply; 30+ messages in thread
From: Eli Zaretskii @ 2019-09-30 14:01 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: monnier, emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Mon, 30 Sep 2019 15:42:11 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I don't think I understand why we can't put the association in
> > auto-mode-alist before running "gm convert".  Please elaborate why.
> 
> If we put (say) .webp into auto-mode-alist, but we can't read .webp
> files, then image-mode will end up issuing an error when called on these
> files.

I thought when image-mode is invoked, the external converter is called
automagically, no?

> Besides, what formats "gm convert" supports changes over time.  By
> consulting it, auto-mode-alist will be automatically updated without us
> having to do anything.

Yes, but I think it's better to have a solution that is 99% correct
and simple, rather than one that is 100% and much more complex.  After
all, this is somewhat obscure use case, at least for now.

> > Not "do nothing", but display an error message saying this image
> > format is unsupported, IMO.
> 
> Yup.  I meant "do nothing" in the image-mode context, which means "show
> the data as raw bytes and signal an error".

Right.



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

* Re: Image-conversion shims
  2019-09-30  4:12           ` Lars Ingebrigtsen
  2019-09-30  7:06             ` Eli Zaretskii
@ 2019-09-30 17:27             ` Stefan Monnier
  2019-10-01 12:08               ` Lars Ingebrigtsen
  2019-10-06 20:58             ` Juri Linkov
  2 siblings, 1 reply; 30+ messages in thread
From: Stefan Monnier @ 2019-09-30 17:27 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, emacs-devel

> I.e., we want ("*.webp" . image-mode) in auto-mode-alist, but we can't
> put it there before we've called "gm convert -list format",

Why?  .webp is an image format, so it should be associated with
image-mode, whether we can display it or not (it's not like the
behavior would be much better if we don't and end up showing it in
fundamental-mode).


        Stefan




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

* Re: Image-conversion shims
  2019-09-30 14:01                 ` Eli Zaretskii
@ 2019-10-01 12:06                   ` Lars Ingebrigtsen
  2019-10-01 12:36                     ` Eli Zaretskii
  0 siblings, 1 reply; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-01 12:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> > I don't think I understand why we can't put the association in
>> > auto-mode-alist before running "gm convert".  Please elaborate why.
>> 
>> If we put (say) .webp into auto-mode-alist, but we can't read .webp
>> files, then image-mode will end up issuing an error when called on these
>> files.
>
> I thought when image-mode is invoked, the external converter is called
> automagically, no?

The user may not have any of the required external converters installed.

>> Besides, what formats "gm convert" supports changes over time.  By
>> consulting it, auto-mode-alist will be automatically updated without us
>> having to do anything.
>
> Yes, but I think it's better to have a solution that is 99% correct
> and simple, rather than one that is 100% and much more complex.  After
> all, this is somewhat obscure use case, at least for now.

It is.  I'm concerned about three things: 1) Long-time maintenance of all
these auto-mode-alist mapping, 2) the less-than ideal user experience of
being presented with image-mode and an error instead of a
fundamental-mode buffer as today (if you don't have the converters), and
3) possibly stomping on a user's own addition to auto-mode-alist (if
the user adds mapping to the end of the alist).

By having these modes being absolutely-the-last-fallback (when the user
has enabled this at all), then we avoid all those problems.

Because the list of formats covered by these converters is huge, and
putting all of these into the default auto-mode-alist would be a
problem.

(image-converter--probe 'graphicsmagick)
=> ("3fr" "8bim" "8bimtext" "8bimwtext" "app1" "app1jpeg" "art" "arw" "avs" "b" "bie" "bigtiff" "bmp" "c" "cals" "caption" "cin" "cmyk" "cmyka" "cr2" "crw" "cur" "cut" "dcm" "dcr" "dcx" "dng" "dpx" "epdf" "epi" "eps" "epsf" "epsi" "ept" "ept2" "ept3" "erf" "exif" "fax" "file" "fits" "fractal" "ftp" "g" "gif" "gif87" "gradient" "gray" "graya" "hrz" "http" "icb" "icc" "icm" "ico" "icon" "identity" "image" "iptc" "iptctext" "iptcwtext" "jbg" "jbig" "jng" "jnx" "jpeg" "jpg" "k" "k25" "kdc" "label" "m" "mac" "map" "mat" "mef" "miff" "mng" "mono" "mpc" "mrw" "msl" "mtv" "mvg" "nef" "null" "o" "orf" "otb" "p7" "pal" "palm" "pam" "pbm" "pcd" "pcds" "pct" "pcx" "pdb" "pdf" "pef" "pfa" "pfb" "pgm" "picon" "pict" "pix" "plasma" "png" "png00" "png24" "png32" "png48" "png64" "png8" "pnm" "ppm" "ps" "ptif" "pwp" "r" "raf" "ras" "rgb" "rgba" "rla" "rle" "sct" "sfw" "sgi" "sr2" "srf" "stegano" "sun" "svg" "svgz" "text" "tga" "tiff" "tile" "tim" "topol" "ttf" "txt" "uyvy" "vda" "vicar" "vid" "viff" "vst" "wbmp" "webp" "wmf" "wpg" "x" "x3f" "xbm" "xc" "xcf" "xmp" "xpm" "xv" "xwd" "y" "yuv")

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-09-30 17:27             ` Stefan Monnier
@ 2019-10-01 12:08               ` Lars Ingebrigtsen
  2019-10-01 13:09                 ` Stefan Monnier
  0 siblings, 1 reply; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-01 12:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

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

>> I.e., we want ("*.webp" . image-mode) in auto-mode-alist, but we can't
>> put it there before we've called "gm convert -list format",
>
> Why?  .webp is an image format, so it should be associated with
> image-mode, whether we can display it or not (it's not like the
> behavior would be much better if we don't and end up showing it in
> fundamental-mode).

For .webp I agree, but there's a bunch of formats that are really
obscure.  It would be nice if Emacs did the right thing if the user has
switched the external image thing on, but some of them might otherwise
be confusing.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-10-01 12:06                   ` Lars Ingebrigtsen
@ 2019-10-01 12:36                     ` Eli Zaretskii
  0 siblings, 0 replies; 30+ messages in thread
From: Eli Zaretskii @ 2019-10-01 12:36 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: monnier, emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org
> Date: Tue, 01 Oct 2019 14:06:55 +0200
> 
> > I thought when image-mode is invoked, the external converter is called
> > automagically, no?
> 
> The user may not have any of the required external converters installed.

In which case they will get an error message, as in the case when they
try visiting a PNG image without libpng compiled in.

> I'm concerned about three things: 1) Long-time maintenance of all
> these auto-mode-alist mapping

I don't think this is a problem, we maintain auto-mode-alist all the
time.

> 2) the less-than ideal user experience of
> being presented with image-mode and an error instead of a
> fundamental-mode buffer as today

Why would a user want Fundamental mode for an image?

> 3) possibly stomping on a user's own addition to auto-mode-alist (if
> the user adds mapping to the end of the alist).

Wouldn't user customizations override the default value?  I think they
would, and then this is not a problem.

> Because the list of formats covered by these converters is huge, and
> putting all of these into the default auto-mode-alist would be a
> problem.

Some of the formats are already supported ("svg", "png", "xpm", etc.),
so they shouldn't be added again; others seem inappropriate or
irrelevant ("http", "file", "ftp", etc.).  As for the length of the
list, I don't think we should worry: the default auto-mode-alist has
188 elements in Emacs 27, so a couple dozen more cannot do much harm,
I think.



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

* Re: Image-conversion shims
  2019-10-01 12:08               ` Lars Ingebrigtsen
@ 2019-10-01 13:09                 ` Stefan Monnier
  0 siblings, 0 replies; 30+ messages in thread
From: Stefan Monnier @ 2019-10-01 13:09 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, emacs-devel

>>> I.e., we want ("*.webp" . image-mode) in auto-mode-alist, but we can't
>>> put it there before we've called "gm convert -list format",
>> Why?  .webp is an image format, so it should be associated with
>> image-mode, whether we can display it or not (it's not like the
>> behavior would be much better if we don't and end up showing it in
>> fundamental-mode).
> For .webp I agree, but there's a bunch of formats that are really
> obscure.

I don't see it makes a difference, except when this obscure format can
only be converted by some equally obscure command.

Assuming a large fraction of Emacs users already have `convert`, gm`, or
`ffmpeg` installed, your code already "enables" those obscure formats,
even though the mere fact that those tools are installed doesn't
indicate that they're likely to use any of those formats.

> It would be nice if Emacs did the right thing if the user has
> switched the external image thing on, but some of them might otherwise
> be confusing.

If they might be confusing when `convert` is not installed, they might
also be confusing when `convert` is installed.


        Stefan




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

* Re: Image-conversion shims
  2019-09-30  4:12           ` Lars Ingebrigtsen
  2019-09-30  7:06             ` Eli Zaretskii
  2019-09-30 17:27             ` Stefan Monnier
@ 2019-10-06 20:58             ` Juri Linkov
  2019-10-07  1:41               ` Lars Ingebrigtsen
  2 siblings, 1 reply; 30+ messages in thread
From: Juri Linkov @ 2019-10-06 20:58 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, monnier, emacs-devel

> I.e., we want ("*.webp" . image-mode) in auto-mode-alist, but we can't
> put it there before we've called "gm convert -list format", and we don't
> do that until we call `create-image' with image-use-external-converter
> non-nil, which won't be called since we don't have a mapping from
> "*.webp" to image-mode.
>
> Very chicken and egg.
>
> Hm...  One possibility would be to do away with
>
> (setq image-use-external-converter t)
>
> as a user-level thing, and instead tell them to do
>
> (image-external-converter-mode)
>
> in ~/.emacs, which would do the probing and then altering
> auto-mode-alist.  Hm.  Perhaps that's a pretty clean idea?  Hm...  I
> think so.  Except that switching the mode off would have to remove the
> entries from auto-mode-alist again, which sounds slightly destructive.

We already have a working solution with imagemagick-register-types,
imagemagick-types-inhibit, imagemagick-enabled-types, and all related code.
Customization should be no different when images are handled via a process
instead of using a linked library.



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

* Re: Image-conversion shims
  2019-10-06 20:58             ` Juri Linkov
@ 2019-10-07  1:41               ` Lars Ingebrigtsen
  2019-10-07 18:48                 ` Juri Linkov
  0 siblings, 1 reply; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-07  1:41 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Eli Zaretskii, monnier, emacs-devel

Juri Linkov <juri@linkov.net> writes:

> We already have a working solution with imagemagick-register-types,
> imagemagick-types-inhibit, imagemagick-enabled-types, and all related code.
> Customization should be no different when images are handled via a process
> instead of using a linked library.

It's quite different -- for one, using the external library is off by
default, but using ImageMagick is on by default (if you have an Emacs
compiled with the support).

But that's pretty much irrelevant for the discussion of
`auto-mode-alist' -- an ImageMagick-enabled Emacs won't use `image-mode'
for exotic image formats by default (you end up in `fundamental-mode' if
you visit a .webp file).  The user has to put that into
`auto-mode-alist' themselves.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-10-07  1:41               ` Lars Ingebrigtsen
@ 2019-10-07 18:48                 ` Juri Linkov
  2019-10-08 16:06                   ` Lars Ingebrigtsen
  0 siblings, 1 reply; 30+ messages in thread
From: Juri Linkov @ 2019-10-07 18:48 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, monnier, emacs-devel

>> We already have a working solution with imagemagick-register-types,
>> imagemagick-types-inhibit, imagemagick-enabled-types, and all related code.
>> Customization should be no different when images are handled via a process
>> instead of using a linked library.
>
> It's quite different -- for one, using the external library is off by
> default, but using ImageMagick is on by default (if you have an Emacs
> compiled with the support).
>
> But that's pretty much irrelevant for the discussion of
> `auto-mode-alist' -- an ImageMagick-enabled Emacs won't use `image-mode'
> for exotic image formats by default (you end up in `fundamental-mode' if
> you visit a .webp file).  The user has to put that into
> `auto-mode-alist' themselves.

After customizing imagemagick-enabled-types to t, it registers all image
types; image-converter could do the same.  This is a good trade-off to
not register all exotic formats in auto-mode-alist by default,
but still easy to customize to stuff everything to auto-mode-alist
when needed.



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

* Re: Image-conversion shims
  2019-10-07 18:48                 ` Juri Linkov
@ 2019-10-08 16:06                   ` Lars Ingebrigtsen
  2019-10-08 17:47                     ` Stefan Monnier
  0 siblings, 1 reply; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-08 16:06 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Eli Zaretskii, monnier, emacs-devel

Juri Linkov <juri@linkov.net> writes:

> After customizing imagemagick-enabled-types to t, it registers all image
> types; image-converter could do the same.  This is a good trade-off to
> not register all exotic formats in auto-mode-alist by default,
> but still easy to customize to stuff everything to auto-mode-alist
> when needed.

Ah, I see.  Yes, it has a

  :set (lambda (symbol value)
	 (set-default symbol value)
	 (imagemagick-register-types))

and we could basically do the same for `image-use-external-converter'.

The only wrinkle is that it would then call the --probe function
typically at Emacs startup (because that's when ~/.emacs is loaded and
`custom-set-variables' is run).  But... the impact of that would be
pretty small ("gm convert -list format" is very fast), and would only
happen for those who have opted in.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-10-08 16:06                   ` Lars Ingebrigtsen
@ 2019-10-08 17:47                     ` Stefan Monnier
  2019-10-09 19:26                       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 30+ messages in thread
From: Stefan Monnier @ 2019-10-08 17:47 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, emacs-devel, Juri Linkov

>> After customizing imagemagick-enabled-types to t, it registers all image
>> types; image-converter could do the same.  This is a good trade-off to
>> not register all exotic formats in auto-mode-alist by default,
>> but still easy to customize to stuff everything to auto-mode-alist
>> when needed.

I still can't see why we should care about our ability to display an
image before preferring image-mode over fundamental-mode.

IOW, I think the auto-mode-alist entries should be added statically not
based on what we can do with them, but based on what the extension means
in practice.

Typically, there are various cases:
- things like .svg and .xpm which can be meaningfully viewed as "an
  image" or as "a text file".  There, we should make sure the user can
  switch between those two views (and default to text if we can't
  display the image, e.g. via image-mode-maybe).
- extensions which are sometimes used for images and sometimes for
  completely unrelated purposes.  We should decide based on
  which is most likely to the case.
- extensions which are always images and can't meaningfully be edited
  within Emacs, so only image-mode makes sense.


        Stefan




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

* Re: Image-conversion shims
  2019-10-08 17:47                     ` Stefan Monnier
@ 2019-10-09 19:26                       ` Lars Ingebrigtsen
  2019-10-09 19:53                         ` Stefan Monnier
  0 siblings, 1 reply; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-09 19:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel, Juri Linkov

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

> I still can't see why we should care about our ability to display an
> image before preferring image-mode over fundamental-mode.

It just feels odd to be mapping file extensions to a mode when we don't
know whether that mode can display the file meaningfully or not.  We
don't do that with other file types, so why with images?

But in practice I guess it doesn't make much difference.  It's more of a
cleanliness thing.

> IOW, I think the auto-mode-alist entries should be added statically not
> based on what we can do with them, but based on what the extension means
> in practice.
>
> Typically, there are various cases:
> - things like .svg and .xpm which can be meaningfully viewed as "an
>   image" or as "a text file".  There, we should make sure the user can
>   switch between those two views (and default to text if we can't
>   display the image, e.g. via image-mode-maybe).
> - extensions which are sometimes used for images and sometimes for
>   completely unrelated purposes.  We should decide based on
>   which is most likely to the case.

Since we don't default to using the external converters for exotic image
formats, when the user has switched it on, that's a signal that the user
does want those to be mapped to images, perhaps, which is a practical
argument for having the mappings be dynamic.

> - extensions which are always images and can't meaningfully be edited
>   within Emacs, so only image-mode makes sense.

In that case we should change image-mode to not display stuff like this
for formats it doesn't understand:

Type C-c C-c or C-c C-x to view the image as an image or hex.
Cannot display image: (Cannot determine image type)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-10-09 19:26                       ` Lars Ingebrigtsen
@ 2019-10-09 19:53                         ` Stefan Monnier
  2019-10-14  4:53                           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 30+ messages in thread
From: Stefan Monnier @ 2019-10-09 19:53 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, emacs-devel, Juri Linkov

>> I still can't see why we should care about our ability to display an
>> image before preferring image-mode over fundamental-mode.
> It just feels odd to be mapping file extensions to a mode when we don't
> know whether that mode can display the file meaningfully or not.

If the result is no worse than what we have in fundamental-mode, I think
it's good.  It's likely going to be better: image-mode could emit
a helpful message explaining which variable to set to enable the use of
external converters or which external converters to install to be able
to display the image, for example.

> We don't do that with other file types, so why with images?

We use `perl-mode` even if there's no `perl` installed ;-)

>> - extensions which are always images and can't meaningfully be edited
>>   within Emacs, so only image-mode makes sense.
>
> In that case we should change image-mode to not display stuff like this
> for formats it doesn't understand:
>
> Type C-c C-c or C-c C-x to view the image as an image or hex.
> Cannot display image: (Cannot determine image type)

Yes, we could improve that.
But it's still no worse than the silent treatment of fundamental-mode.


        Stefan




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

* Re: Image-conversion shims
  2019-10-09 19:53                         ` Stefan Monnier
@ 2019-10-14  4:53                           ` Lars Ingebrigtsen
  2019-10-14 19:27                             ` Juri Linkov
  0 siblings, 1 reply; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-14  4:53 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel, Juri Linkov

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

> If the result is no worse than what we have in fundamental-mode, I think
> it's good.  It's likely going to be better: image-mode could emit
> a helpful message explaining which variable to set to enable the use of
> external converters or which external converters to install to be able
> to display the image, for example.

That's true.  OK, you've convinced me: We should go through the list of
file types the external converters offer and add the ones that make
sense (.webp, .bmp, .etc) to auto-mode-alist, and we should change
image-mode's messaging a bit.

>> We don't do that with other file types, so why with images?
>
> We use `perl-mode` even if there's no `perl` installed ;-)

We do, but perl-mode still works even if there's no perl installed.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

* Re: Image-conversion shims
  2019-10-14  4:53                           ` Lars Ingebrigtsen
@ 2019-10-14 19:27                             ` Juri Linkov
  2019-10-14 19:41                               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 30+ messages in thread
From: Juri Linkov @ 2019-10-14 19:27 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, Stefan Monnier, emacs-devel

>> If the result is no worse than what we have in fundamental-mode, I think
>> it's good.  It's likely going to be better: image-mode could emit
>> a helpful message explaining which variable to set to enable the use of
>> external converters or which external converters to install to be able
>> to display the image, for example.
>
> That's true.  OK, you've convinced me: We should go through the list of
> file types the external converters offer and add the ones that make
> sense (.webp, .bmp, .etc) to auto-mode-alist, and we should change
> image-mode's messaging a bit.

Are file types that make sense the same file types
that are in the default value of imagemagick-enabled-types?

It could be a much shorter list for image-converter, and only
when customized to 't' it could support all possible types.



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

* Re: Image-conversion shims
  2019-10-14 19:27                             ` Juri Linkov
@ 2019-10-14 19:41                               ` Lars Ingebrigtsen
  0 siblings, 0 replies; 30+ messages in thread
From: Lars Ingebrigtsen @ 2019-10-14 19:41 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Eli Zaretskii, Stefan Monnier, emacs-devel

Juri Linkov <juri@linkov.net> writes:

> Are file types that make sense the same file types
> that are in the default value of imagemagick-enabled-types?

(length imagemagick-enabled-types)
=> 112

Sounds a bit long.

> It could be a much shorter list for image-converter, and only
> when customized to 't' it could support all possible types.

Stefan's suggestion is to have a static auto-mode-alist.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



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

end of thread, other threads:[~2019-10-14 19:41 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-28 20:06 Image-conversion shims Lars Ingebrigtsen
2019-09-28 20:37 ` Clément Pit-Claudel
2019-09-28 21:20   ` Lars Ingebrigtsen
2019-09-29  6:24   ` Eli Zaretskii
2019-09-28 21:05 ` Stefan Monnier
2019-09-28 21:21   ` Lars Ingebrigtsen
2019-09-28 23:29   ` Lars Ingebrigtsen
2019-09-29  0:01     ` Lars Ingebrigtsen
2019-09-29  7:11     ` Eli Zaretskii
2019-09-29 11:44       ` Lars Ingebrigtsen
2019-09-29 11:56         ` Eli Zaretskii
2019-09-30  4:12           ` Lars Ingebrigtsen
2019-09-30  7:06             ` Eli Zaretskii
2019-09-30 13:42               ` Lars Ingebrigtsen
2019-09-30 14:01                 ` Eli Zaretskii
2019-10-01 12:06                   ` Lars Ingebrigtsen
2019-10-01 12:36                     ` Eli Zaretskii
2019-09-30 17:27             ` Stefan Monnier
2019-10-01 12:08               ` Lars Ingebrigtsen
2019-10-01 13:09                 ` Stefan Monnier
2019-10-06 20:58             ` Juri Linkov
2019-10-07  1:41               ` Lars Ingebrigtsen
2019-10-07 18:48                 ` Juri Linkov
2019-10-08 16:06                   ` Lars Ingebrigtsen
2019-10-08 17:47                     ` Stefan Monnier
2019-10-09 19:26                       ` Lars Ingebrigtsen
2019-10-09 19:53                         ` Stefan Monnier
2019-10-14  4:53                           ` Lars Ingebrigtsen
2019-10-14 19:27                             ` Juri Linkov
2019-10-14 19:41                               ` Lars Ingebrigtsen

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