bug#25905: 25.1; Can't find regexp and replace in SVG files

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Matthias Meulien]

[-- Attachment #1: Type: text/plain, Size: 3494 bytes --]

Open a dired buffer visiting a directory with SVG images. Mark some SVG
files then try to `dired-do-find-regexp-and-replace' in those files.

It fails due to `image-file-mode' turning all visited buffers as
read-only...



In GNU Emacs 25.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.22.7)
 of 2017-02-07 built on arojas
Windowing system distributor 'The X.Org Foundation', version
11.0.11901000
System Description:	Arch Linux

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe
 -fstack-protector-strong' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LANG: fr_FR.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Image[imagemagick]

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Marking matching files...
48 matching files marked.
Type C-c C-c to view the image as text. [2 times]
Mark set
replace-match-maybe-edit: Text is read-only
Making completion list... [2 times]

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message idna format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns mail-prsvr mail-utils
image-mode find-dired semantic/fw mode-local find-func xref cl-seq
project eieio byte-opt bytecomp byte-compile cconv eieio-core cl-macs
gv
cl-extra help-mode easymenu grep compile comint ansi-color ring
dired-aux cl-loaddefs pcase cl-lib thingatpt dired time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel x-win term/common-win x-dnd tool-bar dnd fontset image regexp-
opt
fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode
register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote dbusbind inotify dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 113568 8337)
 (symbols 48 21910 0)
 (miscs 40 133 179)
 (strings 32 21059 4609)
 (string-bytes 1 676212)
 (vectors 16 15375)
 (vector-slots 8 461447 4393)
 (floats 8 242 81)
 (intervals 56 1459 8)
 (buffers 976 25))

-- 
Matthias Meulien
www.delair-tech.com

[-- Attachment #2: Type: text/html, Size: 3559 bytes --]

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Lars Ingebrigtsen]

Matthias Meulien <matthias.meulien@delair-tech.com> writes:

> Open a dired buffer visiting a directory with SVG images. Mark some SVG
> files then try to `dired-do-find-regexp-and-replace' in those files.
>
> It fails due to `image-file-mode' turning all visited buffers as
> read-only...

(I'm going through old bug reports that unfortunately got no response at
the time.)

It'll also display the svg files as images (although with the real text
in the buffer "shadowed" by the image).

I guess this is a problem with all files that Emacs displays in
... highly interpreted form, like images and tar files and the like.
The thing that makes SVG files special here is that you could see
somebody actually want to do a find-and-replace on them, while you don't
really see that in a, say, zip file.  Often.

But I wonder whether this problem should nevertheless have a general
solution.  That is, should this command have a variation that works on
literal files instead?  Perhaps a prefix of `C-u' to make the query
replace use find-file-literally?

Any opinions?

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

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Juri Linkov]

>> Open a dired buffer visiting a directory with SVG images. Mark some SVG
>> files then try to `dired-do-find-regexp-and-replace' in those files.
>>
>> It fails due to `image-file-mode' turning all visited buffers as
>> read-only...
>
> It'll also display the svg files as images (although with the real text
> in the buffer "shadowed" by the image).
>
> I guess this is a problem with all files that Emacs displays in
> ... highly interpreted form, like images and tar files and the like.
> The thing that makes SVG files special here is that you could see
> somebody actually want to do a find-and-replace on them, while you don't
> really see that in a, say, zip file.  Often.
>
> But I wonder whether this problem should nevertheless have a general
> solution.  That is, should this command have a variation that works on
> literal files instead?  Perhaps a prefix of `C-u' to make the query
> replace use find-file-literally?
>
> Any opinions?

A general solution is to customize image-visiting mode as text
by adding to the init file:

  (push '("\\.svgz?\\'" . image-mode-as-text) auto-mode-alist)

I don't know whether or where this solution could be mentioned more,
but currently this is documented in image-mode-as-text:

  You can use `image-mode-as-text' in `auto-mode-alist' when you want
  to display an image file as text initially.

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> A general solution is to customize image-visiting mode as text
> by adding to the init file:
>
>   (push '("\\.svgz?\\'" . image-mode-as-text) auto-mode-alist)
>
> I don't know whether or where this solution could be mentioned more,
> but currently this is documented in image-mode-as-text:
>
>   You can use `image-mode-as-text' in `auto-mode-alist' when you want
>   to display an image file as text initially.

I don't think that's a general solution -- most people would want Emacs
to display .svg files as images, I think.

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

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Dmitry Gutov]

On 12.12.2020 21:56, Lars Ingebrigtsen wrote:
> But I wonder whether this problem should nevertheless have a general
> solution.  That is, should this command have a variation that works on
> literal files instead?  Perhaps a prefix of `C-u' to make the query
> replace use find-file-literally?

Perhaps there could be a whitelist of such modes instead, with commands 
that can be used to switch buffer contents to "replacable" form?

At least in this particular example, it's hard for me to imagine users 
*not* wanting the replacements work by default. So it makes sense as the 
default behavior.

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 12.12.2020 21:56, Lars Ingebrigtsen wrote:
>> But I wonder whether this problem should nevertheless have a general
>> solution.  That is, should this command have a variation that works on
>> literal files instead?  Perhaps a prefix of `C-u' to make the query
>> replace use find-file-literally?
>
> Perhaps there could be a whitelist of such modes instead, with
> commands that can be used to switch buffer contents to "replacable"
> form?
>
> At least in this particular example, it's hard for me to imagine users
> *not* wanting the replacements work by default. So it makes sense as
> the default behavior.

That's true.  There's probably not a lot of these formats -- svg,
xpm...  uhm...  the pnm formats, possibly?  that are "text-like" enough
that people would want to perform search-replaces on them, so writing a
whitelist would probably not be a big job.

So this command would use that whitelist to bind those formats to
fundamental-mode?  Or it could be on auto-mode-alist form:

(defvar query-replace-overriding-mode-alist
  '(("\\.svgz?\\'" . image-mode-as-text)
    ("\\.x[bp]m\\'" . image-mode-as-text)
    ("\\.p[bpgn]m\\'" . image-mode-as-text)))

That would make this trivial to implement.  Comments?

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

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Dmitry Gutov]

On 13.12.2020 14:42, Lars Ingebrigtsen wrote:
> (defvar query-replace-overriding-mode-alist
>    '(("\\.svgz?\\'" . image-mode-as-text)
>      ("\\.x[bp]m\\'" . image-mode-as-text)
>      ("\\.p[bpgn]m\\'" . image-mode-as-text)))
> 
> That would make this trivial to implement.  Comments?

I don't know about the rest of the implementation, but the above is 
pretty much exactly what I was thinking of, thanks.

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Juri Linkov]

> (defvar query-replace-overriding-mode-alist
>   '(("\\.svgz?\\'" . image-mode-as-text)
>     ("\\.x[bp]m\\'" . image-mode-as-text)
>     ("\\.p[bpgn]m\\'" . image-mode-as-text)))
>
> That would make this trivial to implement.  Comments?

Why only for query-replace?  Shouldn't isearch
display an image file as text for search too?
Maybe this should work like the option 'search-invisible'
when its value is 'open', i.e. to make the searched text
visible - in case of images this means displaying image
buffers as text for search/replace.

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Why only for query-replace?  Shouldn't isearch
> display an image file as text for search too?

Hm...  how would that work?  I'd assume that if you're already in a
buffer with an image, you'd switch to the text version before doing an
isearch.  So I don't quite see how this connects to the
dired-do-query-replace-regexp command...

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

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Juri Linkov]

>> Why only for query-replace?  Shouldn't isearch
>> display an image file as text for search too?
>
> Hm...  how would that work?  I'd assume that if you're already in a
> buffer with an image, you'd switch to the text version before doing an
> isearch.  So I don't quite see how this connects to the
> dired-do-query-replace-regexp command...

There is also the dired-do-isearch command that searches in marked files.

OTOH, when such "making text visible to search" would be implemented
for isearch, then it will be used automatically by query-replace that
relies on the same search functions.

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> There is also the dired-do-isearch command that searches in marked files.
>
> OTOH, when such "making text visible to search" would be implemented
> for isearch, then it will be used automatically by query-replace that
> relies on the same search functions.

That's true...

So we basically need some new functionality for switching to text mode
in image buffers when we're doing searches?  Hm.  Is image mode the only
mode where this would be useful?

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

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Juri Linkov]

>> There is also the dired-do-isearch command that searches in marked files.
>>
>> OTOH, when such "making text visible to search" would be implemented
>> for isearch, then it will be used automatically by query-replace that
>> relies on the same search functions.
>
> That's true...
>
> So we basically need some new functionality for switching to text mode
> in image buffers when we're doing searches?

Maybe isearch-open-necessary-overlays should detect if there is an image
over point, then switch the buffer to text mode.

> Is image mode the only mode where this would be useful?

I don't know any other mode that could benefit from this feature.

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 777 bytes --]

>>> There is also the dired-do-isearch command that searches in marked files.
>>>
>>> OTOH, when such "making text visible to search" would be implemented
>>> for isearch, then it will be used automatically by query-replace that
>>> relies on the same search functions.
>>
>> That's true...
>>
>> So we basically need some new functionality for switching to text mode
>> in image buffers when we're doing searches?
>
> Maybe isearch-open-necessary-overlays should detect if there is an image
> over point, then switch the buffer to text mode.

Here is one possible fix, but I'm still unsure if using image functions
in isearch is a good style.  Perhaps a better fix would be to add this
code to a buffer-local isearch-mode-hook in image-mode.el that runs
when isearch starts.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: inside-image.patch --]
[-- Type: text/x-diff, Size: 653 bytes --]

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 0d5c480c8d..12d31d49a7 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3573,6 +3573,11 @@ isearch-close-unnecessary-overlays
 
 (defun isearch-range-invisible (beg end)
   "Return t if all the text from BEG to END is invisible."
+  (let ((inside-image (and (derived-mode-p 'image-mode)
+                           (fboundp 'image-get-display-property)
+                           (image-get-display-property))))
+    (when (and inside-image (fboundp 'image-mode-as-text))
+      (image-mode-as-text)))
   (when (/= beg end)
     ;; Check that invisibility runs up to END.
     (save-excursion

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

>>>> There is also the dired-do-isearch command that searches in marked files.
>>>>
>>>> OTOH, when such "making text visible to search" would be implemented
>>>> for isearch, then it will be used automatically by query-replace that
>>>> relies on the same search functions.
>>>
>>> That's true...
>>>
>>> So we basically need some new functionality for switching to text mode
>>> in image buffers when we're doing searches?
>>
>> Maybe isearch-open-necessary-overlays should detect if there is an image
>> over point, then switch the buffer to text mode.
>
> Here is one possible fix, but I'm still unsure if using image functions
> in isearch is a good style.  Perhaps a better fix would be to add this
> code to a buffer-local isearch-mode-hook in image-mode.el that runs
> when isearch starts.

Here is a more correct implementation:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: image-mode-isearch-filter.patch --]
[-- Type: text/x-diff, Size: 1064 bytes --]

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 032ebf3873..87a8dc9ef2 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -667,6 +667,9 @@ image-mode--setup-mode
   (when image-auto-resize-on-window-resize
     (add-hook 'window-state-change-functions #'image--window-state-change nil t))
 
+  (add-function :before-while (local 'isearch-filter-predicate)
+                #'image-mode-isearch-filter)
+
   (run-mode-hooks 'image-mode-hook)
   (let ((image (image-get-display-property))
 	(msg1 (substitute-command-keys
@@ -782,6 +790,13 @@ image-toggle-display-text
     (if (called-interactively-p 'any)
 	(message "Repeat this command to go back to displaying the image"))))
 
+(defun image-mode-isearch-filter (_beg _end)
+  "Show image as text when trying to search in the image buffer."
+  (when (and (derived-mode-p 'image-mode)
+             (image-get-display-property))
+    (image-mode-as-text))
+  t)
+
 (defvar archive-superior-buffer)
 (defvar tar-superior-buffer)
 (declare-function image-flush "image.c" (spec &optional frame))

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> +(defun image-mode-isearch-filter (_beg _end)
> +  "Show image as text when trying to search in the image buffer."
> +  (when (and (derived-mode-p 'image-mode)
> +             (image-get-display-property))
> +    (image-mode-as-text))
> +  t)

This would switch to text representation for all images, though.  Is
that what we want?  Hm...  perhaps?

So that's a good solution for isearch, but I don't think we'd want that
for search/replace.  Possibly.

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

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 914 bytes --]

>> +(defun image-mode-isearch-filter (_beg _end)
>> +  "Show image as text when trying to search in the image buffer."
>> +  (when (and (derived-mode-p 'image-mode)
>> +             (image-get-display-property))
>> +    (image-mode-as-text))
>> +  t)
>
> This would switch to text representation for all images, though.  Is
> that what we want?  Hm...  perhaps?

It makes sense to do this even in binary formats when
the user wants to search the text of e.g. EXIF tags, etc.

> So that's a good solution for isearch, but I don't think we'd want that
> for search/replace.  Possibly.

This is what OP wanted (unfortunately, mails to his mailbox bounce back).
But we could install the patch anyway.  The only problem I noticed
that the previous patch shows isearch highlighting at wrong places
because there is a need to wrap the call to image-mode-as-text
in save-match-data (alas, global match data bites again).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: image-mode-isearch-filter.patch --]
[-- Type: text/x-diff, Size: 1091 bytes --]

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 032ebf3873..fdac7164d4 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -667,6 +667,9 @@ image-mode--setup-mode
   (when image-auto-resize-on-window-resize
     (add-hook 'window-state-change-functions #'image--window-state-change nil t))
 
+  (add-function :before-while (local 'isearch-filter-predicate)
+                #'image-mode-isearch-filter)
+
   (run-mode-hooks 'image-mode-hook)
   (let ((image (image-get-display-property))
 	(msg1 (substitute-command-keys
@@ -782,6 +785,14 @@ image-toggle-display-text
     (if (called-interactively-p 'any)
 	(message "Repeat this command to go back to displaying the image"))))
 
+(defun image-mode-isearch-filter (_beg _end)
+  "Show image as text when trying to search in the image buffer."
+  (save-match-data
+    (when (and (derived-mode-p 'image-mode)
+               (image-get-display-property))
+      (image-mode-as-text)))
+  t)
+
 (defvar archive-superior-buffer)
 (defvar tar-superior-buffer)
 (declare-function image-flush "image.c" (spec &optional frame))

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>> So that's a good solution for isearch, but I don't think we'd want that
>> for search/replace.  Possibly.
>
> This is what OP wanted (unfortunately, mails to his mailbox bounce back).
> But we could install the patch anyway.

Sorry, I assumed that isearch-filter-predicate was for isearch only,
but it's for query-replace, too?  In that case, the code looks hunky
dory to me -- go ahead and push.

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

bug#25905: 25.1; Can't find regexp and replace in SVG files

[Juri Linkov]

tags 25905 fixed
close 25905 28.0.50
quit

>>> So that's a good solution for isearch, but I don't think we'd want that
>>> for search/replace.  Possibly.
>>
>> This is what OP wanted (unfortunately, mails to his mailbox bounce back).
>> But we could install the patch anyway.
>
> Sorry, I assumed that isearch-filter-predicate was for isearch only,
> but it's for query-replace, too?

Yep, this is a quite recent addition that query-replace relies
on all isearch bells and whistles for doing its search part.

> In that case, the code looks hunky dory to me -- go ahead and push.

So pushed to master.