unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#9516: imagemagick-register-types and image-file-name-extensions
@ 2011-09-15 18:34 Juri Linkov
  2011-09-16  2:23 ` Stefan Monnier
  2011-09-16 13:34 ` Stefan Monnier
  0 siblings, 2 replies; 10+ messages in thread
From: Juri Linkov @ 2011-09-15 18:34 UTC (permalink / raw)
  To: 9516

In Dired typing `C-t C-t' (`image-dired-dired-toggle-marked-thumbs')
doesn't show images supported by ImageMagick.

It gets file extensions of supported image types by calling the function
`image-file-name-regexp' and it gets them from the variable
`image-file-name-extensions' that currently misses file extensions
supported by ImageMagick.

I propose a fix that adds them to `image-file-name-extensions'
in `imagemagick-register-types':

=== modified file 'lisp/image.el'
--- lisp/image.el	2011-07-25 08:23:29 +0000
+++ lisp/image.el	2011-09-15 18:32:51 +0000
@@ -699,11 +699,11 @@ (defun imagemagick-register-types ()
       (dolist (im-inhibit imagemagick-types-inhibit)
 	(setq im-types (delq im-inhibit im-types)))
       (dolist (im-type im-types)
-	(let ((extension
-	       (concat "\\." (downcase (symbol-name im-type))
-		       "\\'")))
-	  (push (cons extension 'image-mode) auto-mode-alist)
-	  (push (cons extension 'imagemagick)
+	(let* ((extension (downcase (symbol-name im-type)))
+	       (extension-regexp (concat "\\." extension "\\'")))
+	  (push extension image-file-name-extensions)
+	  (push (cons extension-regexp 'image-mode) auto-mode-alist)
+	  (push (cons extension-regexp 'imagemagick)
 		image-type-file-name-regexps))))))

There are other packages that rely on image file extensions
defined in image-file.el:

./iimage.el:61:				     image-file-name-extensions)
./iimage.el:62:			     image-file-name-extensions)
./image-dired.el:548:  (unless (string-match (image-file-name-regexp) file)
./image-dired.el:656:     (when (and image-file (string-match-p (image-file-name-regexp) image-file))
./image-dired.el:867:  (dired-mark-files-regexp (image-file-name-regexp))
./org/org.el:19319:  (if (and (not extensions) (fboundp 'image-file-name-regexp))
./org/org.el:19320:      (image-file-name-regexp)
./org/org.el:19321:    (let ((image-file-name-extensions
./org/org.el:19327:					 image-file-name-extensions)
./org/org.el:19328:				 image-file-name-extensions)
./thumbs.el:228:	    (directory-files (thumbs-thumbsdir) t (image-file-name-regexp)))
./thumbs.el:411:   (directory-files dir t (or reg (image-file-name-regexp)))

I suppose all they will benefit from this fix as well.





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

* bug#9516: imagemagick-register-types and image-file-name-extensions
  2011-09-15 18:34 bug#9516: imagemagick-register-types and image-file-name-extensions Juri Linkov
@ 2011-09-16  2:23 ` Stefan Monnier
  2011-09-16 13:34 ` Stefan Monnier
  1 sibling, 0 replies; 10+ messages in thread
From: Stefan Monnier @ 2011-09-16  2:23 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 9516

> +	       (extension-regexp (concat "\\." extension "\\'")))

You forgot a regexp-quote.


        Stefan





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

* bug#9516: imagemagick-register-types and image-file-name-extensions
  2011-09-15 18:34 bug#9516: imagemagick-register-types and image-file-name-extensions Juri Linkov
  2011-09-16  2:23 ` Stefan Monnier
@ 2011-09-16 13:34 ` Stefan Monnier
  2011-09-16 14:09   ` Juri Linkov
  1 sibling, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2011-09-16 13:34 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 9516

> I propose a fix that adds them to `image-file-name-extensions'
> in `imagemagick-register-types':

There's something I don't understand:
- why do we have both image-file-name-extensions and image-file-name-regexps?
- why doesn't image-file.el use image-type-file-name-regexps?


        Stefan





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

* bug#9516: imagemagick-register-types and image-file-name-extensions
  2011-09-16 13:34 ` Stefan Monnier
@ 2011-09-16 14:09   ` Juri Linkov
  2011-09-23 21:34     ` Chong Yidong
  0 siblings, 1 reply; 10+ messages in thread
From: Juri Linkov @ 2011-09-16 14:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 9516

>> I propose a fix that adds them to `image-file-name-extensions'
>> in `imagemagick-register-types':
>
> There's something I don't understand:
> - why do we have both image-file-name-extensions and image-file-name-regexps?

I guess for historical reasons.

> - why doesn't image-file.el use image-type-file-name-regexps?

It seems image-file.el is semi-obsoleted by image-mode.el,
so it should use more general settings from image-type-file-name-regexps.
And other packages should be fixed to use more general settings
from image.el instead of relying on definitions from image-file.el.





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

* bug#9516: imagemagick-register-types and image-file-name-extensions
  2011-09-16 14:09   ` Juri Linkov
@ 2011-09-23 21:34     ` Chong Yidong
  2011-09-23 21:40       ` Chong Yidong
  0 siblings, 1 reply; 10+ messages in thread
From: Chong Yidong @ 2011-09-23 21:34 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 9516

Juri Linkov <juri@jurta.org> writes:

> It seems image-file.el is semi-obsoleted by image-mode.el,
> so it should use more general settings from image-type-file-name-regexps.
> And other packages should be fixed to use more general settings
> from image.el instead of relying on definitions from image-file.el.

Sounds right, though I think that can wait till post-release.

Please go ahead and commit your patch (with the regexp-quote).





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

* bug#9516: imagemagick-register-types and image-file-name-extensions
  2011-09-23 21:34     ` Chong Yidong
@ 2011-09-23 21:40       ` Chong Yidong
  2011-09-25 14:03         ` Juri Linkov
  0 siblings, 1 reply; 10+ messages in thread
From: Chong Yidong @ 2011-09-23 21:40 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 9516

Chong Yidong <cyd@stupidchicken.com> writes:

> Please go ahead and commit your patch (with the regexp-quote).

On second thought, there is a problem---image-file-name-extensions is a
defcustom, so your patch would trigger a "changed outside customize"
warning if the user tries customizing it after calling
imagemagick-register-types.





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

* bug#9516: imagemagick-register-types and image-file-name-extensions
  2011-09-23 21:40       ` Chong Yidong
@ 2011-09-25 14:03         ` Juri Linkov
  2019-09-25 12:30           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Juri Linkov @ 2011-09-25 14:03 UTC (permalink / raw)
  To: Chong Yidong; +Cc: 9516

>> Please go ahead and commit your patch (with the regexp-quote).
>
> On second thought, there is a problem---image-file-name-extensions is a
> defcustom, so your patch would trigger a "changed outside customize"
> warning if the user tries customizing it after calling
> imagemagick-register-types.

Then we could change the function `image-file-name-regexp'
that dynamically constructs a composite regexp from
`image-file-name-extensions' and `image-file-name-regexps'.
What we could do is to add to its return value a regexp added
in `imagemagick-register-types' to `image-type-file-name-regexps'
with the assoc value `imagemagick':

=== modified file 'lisp/image-file.el'
--- lisp/image-file.el	2011-01-25 04:08:28 +0000
+++ lisp/image-file.el	2011-09-25 14:01:31 +0000
@@ -85,13 +85,12 @@ (defun image-file-name-regexp ()
 					 image-file-name-extensions)
 				  t)
 		      "\\'"))))
-    (if image-file-name-regexps
-	(mapconcat 'identity
-		   (if exts-regexp
-		       (cons exts-regexp image-file-name-regexps)
-		     image-file-name-regexps)
-		   "\\|")
-      exts-regexp)))
+    (mapconcat
+     'identity
+     (delq nil (list exts-regexp
+		     image-file-name-regexps
+		     (car (rassq 'imagemagick image-type-file-name-regexps))))
+     "\\|")))

This is a temporary solution for the next release of 24.1 that
will allow image-dired.el to display Imagemagick supported images.

I think in 24.2, defcustoms in image-file.el should be deprecated
and packages should use `image-type-file-name-regexps' instead.





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

* bug#9516: imagemagick-register-types and image-file-name-extensions
  2011-09-25 14:03         ` Juri Linkov
@ 2019-09-25 12:30           ` Lars Ingebrigtsen
  2019-09-25 19:57             ` Juri Linkov
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-25 12:30 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 9516, Chong Yidong

Juri Linkov <juri@jurta.org> writes:

> Then we could change the function `image-file-name-regexp'
> that dynamically constructs a composite regexp from
> `image-file-name-extensions' and `image-file-name-regexps'.
> What we could do is to add to its return value a regexp added
> in `imagemagick-register-types' to `image-type-file-name-regexps'
> with the assoc value `imagemagick':

[...]

> +    (mapconcat
> +     'identity
> +     (delq nil (list exts-regexp
> +		     image-file-name-regexps
> +		     (car (rassq 'imagemagick image-type-file-name-regexps))))
> +     "\\|")))
>
> This is a temporary solution for the next release of 24.1 that
> will allow image-dired.el to display Imagemagick supported images.
>
> I think in 24.2, defcustoms in image-file.el should be deprecated
> and packages should use `image-type-file-name-regexps' instead.

This was seven years ago, but the patch wasn't applied.  I know that
ImageMagick is slightly semi-deprecated now, but I think this patch
possibly makes sense anyway.

I don't use the Dired thumbnail stuff, though.  Did anybody have an
objection to this that didn't land in the bug tracker?

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





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

* bug#9516: imagemagick-register-types and image-file-name-extensions
  2019-09-25 12:30           ` Lars Ingebrigtsen
@ 2019-09-25 19:57             ` Juri Linkov
  2019-09-26 19:27               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Juri Linkov @ 2019-09-25 19:57 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 9516

>> Then we could change the function `image-file-name-regexp'
>> that dynamically constructs a composite regexp from
>> `image-file-name-extensions' and `image-file-name-regexps'.
>> What we could do is to add to its return value a regexp added
>> in `imagemagick-register-types' to `image-type-file-name-regexps'
>> with the assoc value `imagemagick':
>
> [...]
>
>> +    (mapconcat
>> +     'identity
>> +     (delq nil (list exts-regexp
>> +		     image-file-name-regexps
>> +		     (car (rassq 'imagemagick image-type-file-name-regexps))))
>> +     "\\|")))
>>
>> This is a temporary solution for the next release of 24.1 that
>> will allow image-dired.el to display Imagemagick supported images.
>>
>> I think in 24.2, defcustoms in image-file.el should be deprecated
>> and packages should use `image-type-file-name-regexps' instead.
>
> This was seven years ago, but the patch wasn't applied.  I know that
> ImageMagick is slightly semi-deprecated now, but I think this patch
> possibly makes sense anyway.

There was no hurry in installing this because all widely used formats
are already presented in image-type-file-name-regexps.

> I don't use the Dired thumbnail stuff, though.  Did anybody have an
> objection to this that didn't land in the bug tracker?

Sorry, I can't confirm if the patch still works since I don't remember
how I tested it long ago, so not sure if this change might break something.





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

* bug#9516: imagemagick-register-types and image-file-name-extensions
  2019-09-25 19:57             ` Juri Linkov
@ 2019-09-26 19:27               ` Lars Ingebrigtsen
  0 siblings, 0 replies; 10+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-26 19:27 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 9516

Juri Linkov <juri@jurta.org> writes:

>> I don't use the Dired thumbnail stuff, though.  Did anybody have an
>> objection to this that didn't land in the bug tracker?
>
> Sorry, I can't confirm if the patch still works since I don't remember
> how I tested it long ago, so not sure if this change might break something.

I've now done some light testing in an ImageMagick-enabled Emacs, and it
seems to work fine, so I've pushed the patch.

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





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

end of thread, other threads:[~2019-09-26 19:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-15 18:34 bug#9516: imagemagick-register-types and image-file-name-extensions Juri Linkov
2011-09-16  2:23 ` Stefan Monnier
2011-09-16 13:34 ` Stefan Monnier
2011-09-16 14:09   ` Juri Linkov
2011-09-23 21:34     ` Chong Yidong
2011-09-23 21:40       ` Chong Yidong
2011-09-25 14:03         ` Juri Linkov
2019-09-25 12:30           ` Lars Ingebrigtsen
2019-09-25 19:57             ` Juri Linkov
2019-09-26 19:27               ` 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).