[PATCH] iimage-mode: reset point for each regexp

[PATCH] iimage-mode: reset point for each regexp

[LensPlaysGames]

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

When creating a minor mode that would replace certain regexp matches
with inline images, I was glad to find iimage-mode already existed to
make this process easier; better yet, it's built in to Emacs core.
However, upon customising the functionality of this mode, by altering
iimage-mode-image-regex-alist to contain more than one item, I found
an interesting quirk of the mode's behaviour. If a regular expression
early in the list matched late in the buffer, then regular expression
matches later in the list would no longer replace matches early in the
buffer. I didn't expect the alist to have such strict ordering, and
I'm not sure if that's intentional, especially as the defaults have
only one element in this alist.

When peering at the source code, I didn't notice any documentation
indicating that this behaviour is expected. I also noticed the cause
of this behaviour: '(goto-char (point-min))' is used outside of the
'dolist' iteration that loops over regular expressions to match, which
means that each regexp search begins at the end of the last... By
moving this goto-char call one line down, within the 'dolist' body,
each regexp search begins at, well, the beginning. This gives expected
behaviour in my minor mode (such that an alphabetized list of exact
matches can have all occurrences replaced with inline images in a
buffer). If this behaviour is intended, feel free to ignore this post.

Attached is a patch generated with git that implements the above change.

[-- Attachment #2: iimage.patch --]
[-- Type: application/octet-stream, Size: 558 bytes --]

diff --git a/lisp/iimage.el b/lisp/iimage.el
index 96ab963bff..053eb27db7 100644
--- a/lisp/iimage.el
+++ b/lisp/iimage.el
@@ -112,8 +112,8 @@ Examples of image filename patterns to match:
 	file)
     (with-silent-modifications
       (save-excursion
-        (goto-char (point-min))
         (dolist (pair iimage-mode-image-regex-alist)
+          (goto-char (point-min))
           (while (re-search-forward (car pair) nil t)
             (when (and (setq file (match-string (cdr pair)))
                        (setq file (locate-file file image-path)))

Re: [PATCH] iimage-mode: reset point for each regexp

[Juri Linkov]

> When creating a minor mode that would replace certain regexp matches
> with inline images, I was glad to find iimage-mode already existed to
> make this process easier; better yet, it's built in to Emacs core.
> However, upon customising the functionality of this mode, by altering
> iimage-mode-image-regex-alist to contain more than one item, I found
> an interesting quirk of the mode's behaviour. If a regular expression
> early in the list matched late in the buffer, then regular expression
> matches later in the list would no longer replace matches early in the
> buffer. I didn't expect the alist to have such strict ordering, and
> I'm not sure if that's intentional, especially as the defaults have
> only one element in this alist.
>
> When peering at the source code, I didn't notice any documentation
> indicating that this behaviour is expected. I also noticed the cause
> of this behaviour: '(goto-char (point-min))' is used outside of the
> 'dolist' iteration that loops over regular expressions to match, which
> means that each regexp search begins at the end of the last... By
> moving this goto-char call one line down, within the 'dolist' body,
> each regexp search begins at, well, the beginning. This gives expected
> behaviour in my minor mode (such that an alphabetized list of exact
> matches can have all occurrences replaced with inline images in a
> buffer). If this behaviour is intended, feel free to ignore this post.
>
> Attached is a patch generated with git that implements the above change.

Thanks for the bug report.  I'm using iimage-mode-image-regex-alist
with a single composite regexp, so I never noticed this problem.

Eli, is it ok to push this fix to the emacs-29 branch?

> diff --git a/lisp/iimage.el b/lisp/iimage.el
> index 96ab963bff..053eb27db7 100644
> --- a/lisp/iimage.el
> +++ b/lisp/iimage.el
> @@ -112,8 +112,8 @@ Examples of image filename patterns to match:
>  	file)
>      (with-silent-modifications
>        (save-excursion
> -        (goto-char (point-min))
>          (dolist (pair iimage-mode-image-regex-alist)
> +          (goto-char (point-min))
>            (while (re-search-forward (car pair) nil t)
>              (when (and (setq file (match-string (cdr pair)))
>                         (setq file (locate-file file image-path)))

Re: [PATCH] iimage-mode: reset point for each regexp

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: LensPlaysGames <lensplaysgames@gmail.com>,  emacs-devel@gnu.org
> Date: Wed, 08 Feb 2023 09:29:37 +0200
> 
> Thanks for the bug report.  I'm using iimage-mode-image-regex-alist
> with a single composite regexp, so I never noticed this problem.
> 
> Eli, is it ok to push this fix to the emacs-29 branch?

Yes, thanks.  Just don't forget the "Copyright-paperwork-exempt"
thingy.