Re: [Emacs-diffs] emacs/lisp image.el

Re: [Emacs-diffs] emacs/lisp image.el

[Juanma Barranquero]

On Sat, Aug 29, 2009 at 01:49, Juri Linkov<juri@jurta.org> wrote:

> Log message:
>        (image-type-header-regexps): Use only JPEG magic number
>        to determine JPEG images, and don't use `image-jpeg-p' because
>        Emacs can display non-JFIF non-Exif JPEG images.

Then we should perhaps obsolete `image-jpeg-p' (which is not used
anymore in the sources).

Currently it does what its docstring says, but there's some of a
mismatch with the name (which seems to imply more generality); also it
is not clear what purpose does serve a function to detect just
JFIF/Exif JPEGs. And generalizing it would turn it into a trivial
two-liner.

    Juanma

Re: [Emacs-diffs] emacs/lisp image.el

[Juri Linkov]

>> Log message:
>>        (image-type-header-regexps): Use only JPEG magic number
>>        to determine JPEG images, and don't use `image-jpeg-p' because
>>        Emacs can display non-JFIF non-Exif JPEG images.
>
> Then we should perhaps obsolete `image-jpeg-p' (which is not used
> anymore in the sources).
>
> Currently it does what its docstring says, but there's some of a
> mismatch with the name (which seems to imply more generality); also it
> is not clear what purpose does serve a function to detect just
> JFIF/Exif JPEGs. And generalizing it would turn it into a trivial
> two-liner.

I left `image-jpeg-p' in image.el for the case it's used by some
external packages or .emacs files since it contains a non-trivial
algorithm for detecting JFIF/Exif JPEGs.  Even though its name is
not too specific, its docstring explains what it does.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [Emacs-diffs] emacs/lisp image.el

[Juanma Barranquero]

On Sun, Aug 30, 2009 at 00:46, Juri Linkov<juri@jurta.org> wrote:

> I left `image-jpeg-p' in image.el for the case it's used by some
> external packages or .emacs files since it contains a non-trivial
> algorithm for detecting JFIF/Exif JPEGs.  Even though its name is
> not too specific, its docstring explains what it does.

IMO, it is inconsistent with what image.el (or indeed Emacs) offers;
we don't have functions to detect the different image formats
supported by TIFF, for example.

`image-jpeg-p' was never a published interface, just a helper
function. As a very crude test, googling for it brings a few hits
(plus "image/jpeg" false hits), most of which seem to be from our code
or bug reports. It's unlikely the function has received wide usage
(yes, private libraries and most .emacs files do not appear in Google;
I said "crude").

If removing it is too severe, at least obsoleting it seems to be in
order. It is unused in the sources, too specific, inconsistently
documented, and internal.

    Juanma

Re: [Emacs-diffs] emacs/lisp image.el

[Juri Linkov]

> IMO, it is inconsistent with what image.el (or indeed Emacs) offers;
> we don't have functions to detect the different image formats
> supported by TIFF, for example.
>
> `image-jpeg-p' was never a published interface, just a helper
> function. As a very crude test, googling for it brings a few hits
> (plus "image/jpeg" false hits), most of which seem to be from our code
> or bug reports. It's unlikely the function has received wide usage
> (yes, private libraries and most .emacs files do not appear in Google;
> I said "crude").
>
> If removing it is too severe, at least obsoleting it seems to be in
> order. It is unused in the sources, too specific, inconsistently
> documented, and internal.

Then I agree that obsoleting it won't cause any harm.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [Emacs-diffs] emacs/lisp image.el

[Chong Yidong]

Juanma Barranquero <lekktu@gmail.com> writes:

> On Sat, Aug 29, 2009 at 01:49, Juri Linkov<juri@jurta.org> wrote:
>
> Then we should perhaps obsolete `image-jpeg-p' (which is not used
> anymore in the sources).
>
> Currently it does what its docstring says, but there's some of a
> mismatch with the name (which seems to imply more generality); also it
> is not clear what purpose does serve a function to detect just
> JFIF/Exif JPEGs. And generalizing it would turn it into a trivial
> two-liner.

Gerd, do you remember the original rationale for image-jpeg-p?

Re: [Emacs-diffs] emacs/lisp image.el

[Stefan Monnier]

>> Then we should perhaps obsolete `image-jpeg-p' (which is not used
>> anymore in the sources).
>> 
>> Currently it does what its docstring says, but there's some of a
>> mismatch with the name (which seems to imply more generality); also it
>> is not clear what purpose does serve a function to detect just
>> JFIF/Exif JPEGs. And generalizing it would turn it into a trivial
>> two-liner.

> Gerd, do you remember the original rationale for image-jpeg-p?

I have a the vague impression that it's linked to the long discussion we
had about safety and other such things linked to the use of magic-* in
preference to auto-mode-alist, so we wanted to avoid false positives.


        Stefan

Re: [Emacs-diffs] emacs/lisp image.el

[Juanma Barranquero]

On Tue, Sep 1, 2009 at 01:49, Stefan Monnier<monnier@iro.umontreal.ca> wrote:

> I have a the vague impression that it's linked to the long discussion we
> had about safety and other such things linked to the use of magic-* in
> preference to auto-mode-alist, so we wanted to avoid false positives.

But image-jpeg-p exists, and has been used in image-type-regexps, since 2001:

2001-08-08  Gerd Moellmann  <gerd@gnu.org>

        * image.el (image-type-regexps): Allow predicates.  Change the way
        JPEG images are recognized.
        (image-jpeg-p): New function.
        (image-type-from-data): Handle predicates in image-type-regexps.

It was, apparently, never intended to be anything but a helper for
image-type-regexps.

The discussion you're talking about is much more recent, isn't? (2005
or 2006, I think.)

    Juanma