emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Image-conversion shims


From: Eli Zaretskii
Subject: Re: Image-conversion shims
Date: Sun, 29 Sep 2019 14:56:51 +0300

> From: Lars Ingebrigtsen <address@hidden>
> Cc: address@hidden,  address@hidden
> 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?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]