Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

[Yikai Zhao]

Remember to cover the basics, that is, what you expected to happen and
what in fact did happen.  You don't know how to make a good report?  See

     https://orgmode.org/manual/Feedback.html#Feedback

Your bug report will be posted to the Org mailing list.
------------------------------------------------------------------------

Hello org-mode team,

I encountered the following error while opening an org buffer:

  Error during redisplay: (jit-lock-function 1) signaled (wrong-type-argument number-or-marker-p nil)

Here's the minimum reproducable content of the buffer:

  #+BEGIN_SRC markdown
  #+END_SRC

(that is, just an empty markdown source block).

I have markdown-mode https://jblevins.org/projects/markdown-mode/
installed.

After some debugging, I've found that the error happened in
`font-lock-fontify-keywords-region` function. It calls
`org-fontify-meta-lines-and-blocks` and got `t` as the result, but the
matching data is not set.

The following advice can fix the bug for me:

  (defun my/fix-org-fontify-meta-lines-and-blocks (res)
    (when (and res (null (match-beginning 0)))
      (set-match-data (list 0 0)))
    res)
  
  (advice-add 'org-fontify-meta-lines-and-blocks
              :filter-return
              #'my/fix-org-fontify-meta-lines-and-blocks)


Emacs  : GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.17.4)
 of 2021-09-15
Package: Org mode version 9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)

current state:
==============
(setq
 org-link-elisp-confirm-function 'yes-or-no-p
 org-directory "~/Notes"
 org-bibtex-headline-format-function #[257 "\300\236A\207" [:title] 3 "\n\n(fn ENTRY)"]
 org-agenda-files '("~/Notes/gtd/")
 org-export-before-parsing-hook '(org-attach-expand-links)
 org-refile-use-outline-path t
 org-archive-hook '(org-attach-archive-delete-maybe)
 org-cycle-hook '(org-cycle-hide-archived-subtrees org-cycle-hide-drawers
                  org-cycle-show-empty-lines org-optimize-window-after-visibility-change)
 org-mode-hook '(#[0 "\301\211\207" [imenu-create-index-function org-imenu-get-tree] 2]
                 #[0 "\300\301\302\303\304$\207"
                   [add-hook change-major-mode-hook org-show-all append local] 5]
                 #[0 "\300\301\302\303\304$\207"
                   [add-hook change-major-mode-hook org-babel-show-result-all append local]
                   5]
                 org-babel-result-hide-spec org-babel-hide-all-hashes)
 org-confirm-shell-link-function 'yes-or-no-p
 outline-isearch-open-invisible-function 'outline-isearch-open-invisible
 org-agenda-before-write-hook '(org-agenda-add-entry-text)
 org-src-mode-hook '(org-src-babel-configure-edit-buffer org-src-mode-configure-edit-buffer)
 org-confirm-elisp-link-function 'yes-or-no-p
 org-clock-out-hook '(org-clock-remove-empty-clock-drawer)
 org-speed-command-hook '(org-speed-command-activate org-babel-speed-command-activate)
 org-tab-first-hook '(org-babel-hide-result-toggle-maybe org-babel-header-arg-expand)
 org-link-shell-confirm-function 'yes-or-no-p
 org-babel-pre-tangle-hook '(save-buffer)
 org-agenda-loop-over-headlines-in-active-region nil
 org-occur-hook '(org-first-headline-recenter)
 org-metadown-hook '(org-babel-pop-to-session-maybe)
 org-link-parameters '(("attachment" :follow org-attach-follow :complete
                        org-attach-complete-link)
                       ("id" :follow org-id-open)
                       ("treemacs" :store treemacs-store-org-link)
                       ("eww" :follow org-eww-open :store org-eww-store-link)
                       ("rmail" :follow org-rmail-open :store org-rmail-store-link)
                       ("mhe" :follow org-mhe-open :store org-mhe-store-link)
                       ("irc" :follow org-irc-visit :store org-irc-store-link :export
                        org-irc-export)
                       ("info" :follow org-info-open :export org-info-export :store
                        org-info-store-link)
                       ("gnus" :follow org-gnus-open :store org-gnus-store-link)
                       ("docview" :follow org-docview-open :export org-docview-export :store
                        org-docview-store-link)
                       ("bibtex" :follow org-bibtex-open :store org-bibtex-store-link)
                       ("bbdb" :follow org-bbdb-open :export org-bbdb-export :complete
                        org-bbdb-complete-link :store org-bbdb-store-link)
                       ("w3m" :store org-w3m-store-link) ("file+sys") ("file+emacs")
                       ("shell" :follow org-link--open-shell)
                       ("news" :follow
                        #[514 "\301\300\302Q\"\207" ["news" browse-url ":"] 6
                          "\n\n(fn URL ARG)"]
                        )
                       ("mailto" :follow
                        #[514 "\301\300\302Q\"\207" ["mailto" browse-url ":"] 6
                          "\n\n(fn URL ARG)"]
                        )
                       ("https" :follow
                        #[514 "\301\300\302Q\"\207" ["https" browse-url ":"] 6
                          "\n\n(fn URL ARG)"]
                        )
                       ("http" :follow
                        #[514 "\301\300\302Q\"\207" ["http" browse-url ":"] 6
                          "\n\n(fn URL ARG)"]
                        )
                       ("ftp" :follow
                        #[514 "\301\300\302Q\"\207" ["ftp" browse-url ":"] 6
                          "\n\n(fn URL ARG)"]
                        )
                       ("help" :follow org-link--open-help)
                       ("file" :complete org-link-complete-file)
                       ("elisp" :follow org-link--open-elisp)
                       ("doi" :follow org-link--open-doi))
 org-metaup-hook '(org-babel-load-in-session-maybe)
 org-outline-path-complete-in-steps nil
 )

Re: Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

[Timothy]

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

Hi Yikai,

Thanks you for reporting this bug. I was able to confirm it, and trying a few
similar such buffers can also say this only appears to affect `markdown-mode'.

I’ve prepared a patch that serves as a catch-all for this type of issue, but I’m
not sure if this is the best approach. If other people could chime in that would
be great :)

All the best,
Timothy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-fix-potential-missing-src-match-data-issue.patch --]
[-- Type: text/x-patch, Size: 834 bytes --]

From d74d588a004b36d67c603ccbd919e07192524281 Mon Sep 17 00:00:00 2001
From: TEC <tec@tecosaur.com>
Date: Fri, 17 Sep 2021 17:23:58 +0800
Subject: [PATCH] org: fix potential missing src match-data issue

A more detailed commit message if this actually gets applied...
---
 lisp/org.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/org.el b/lisp/org.el
index c22a69154..4c630f206 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -5377,6 +5377,8 @@ (defun org-fontify-meta-lines-and-blocks-1 (limit)
 		     (min (point-max) beg-of-next-line))
 		 (min (point-max) end-of-endline))
 	       '(face org-block-end-line)))
+            (unless (match-data)
+              (set-match-data '(0 0)))
 	    t))
 	 ((member dc1 '("+title:" "+subtitle:" "+author:" "+email:" "+date:"))
 	  (org-remove-flyspell-overlays-in
-- 
2.33.0

Re: Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

[Timothy]

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

Just as a little bump/update.

My current thoughts on this are that this is probably an issue with
`markdown-mode', but it would be good if issues with langs’ modes used in Org src
blocks weren’t able to mess with Org’s fontification, and so this patch[1] or one
like it should be merged.

All the best,
Timothy



Footnotes
─────────

[1] The comment message is still a placeholder as of this moment, but that’s
easy enough to change

Re: Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

[Bastien]

Hi Timothy,

Timothy <tecosaur@gmail.com> writes:

> I’ve prepared a patch that serves as a catch-all for this type of issue, but I’m
> not sure if this is the best approach. 

Can you commit a proper patch to the bugfix branch, with a comment in
the code explaining why this is a catch-all hack?

Also, if someone can report the bug upstream for markdown-mode, that
be great.

Thanks!

-- 
 Bastien

Re: Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

[Timothy]

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

Hi  Bastien,

> Can you commit a proper patch to the bugfix branch, with a comment in
> the code explaining why this is a catch-all hack?

I’ve just written a proper commit message for this, and in the process realised
that I wasn’t actually dealing with this the best way — and so amended my fix
to something I’m fairly happy with. I haven’t pushed to the bugfix branch
before, should I immediately merge the change into master too, or just leave it
in bugfix only?

All the best,
Timothy

Re: Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

[Bastien]

Hi Timothy,

Timothy <tecosaur@gmail.com> writes:

> I’ve just written a proper commit message for this, and in the
> process realised that I wasn’t actually dealing with this the best
> way — and so amended my fix to something I’m fairly happy with.

Okay - can you post the patch in this thread?

> I haven’t pushed to the bugfix branch before, should I immediately
> merge the change into master too, or just leave it in bugfix only?

After you send it here, you can push to bugfix and merge the bugfix
branch into main.

You can also add the X-Woof-Patch: applied header to your reply.

Thanks,

-- 
 Bastien

Re: Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

[Timothy]

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

Hi  Bastien,

> Okay - can you post the patch in this thread?

Ahh, I forgot to attach it didn’t I. You should find it attached to this message.

> After you send it here, you can push to bugfix and merge the bugfix
> branch into main.

I’ll do exactly this shortly.

> You can also add the X-Woof-Patch: applied header to your reply.

I fully intend to 😀. In fact, I’ve recently made this much easier for myself
(<https://tecosaur.github.io/emacs-config/config.html#adding-x-woof>).
In case this is of interest:
<file:~/Desktop/xwoof-tool.png>

All the best,
Timothy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-Fix-potential-modified-src-match-data-issue.patch --]
[-- Type: text/x-patch, Size: 1353 bytes --]

From 0c3571097bda58d1ef0fd0554322a42ff7068a67 Mon Sep 17 00:00:00 2001
From: TEC <tec@tecosaur.com>
Date: Fri, 17 Sep 2021 17:23:58 +0800
Subject: [PATCH 1/6] org: Fix potential modified src match-data issue

* lisp/org.el (org-fontify-meta-lines-and-blocks-1): When this is run on
a src block, a "leaky" major mode called in
`org-src-font-lock-fontify-block' can modify the match data.
This is problematic, as the match data already set is important for
font-lock.  To protect ourselves from this behaviour, we can wrap
`org-src-font-lock-fontify-block' in `save-match-data' to ensure that
the match data for the src block is conserved.
---
 lisp/org.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index 733dda429..64f82b5ee 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -5330,7 +5330,8 @@ (defun org-fontify-meta-lines-and-blocks-1 (limit)
 	    (org-remove-flyspell-overlays-in nl-before-endline end-of-endline)
 	    (cond
 	     ((and lang (not (string= lang "")) org-src-fontify-natively)
-	      (org-src-font-lock-fontify-block lang block-start block-end)
+	      (save-match-data
+                (org-src-font-lock-fontify-block lang block-start block-end))
 	      (add-text-properties bol-after-beginline block-end '(src-block t)))
 	     (quoting
 	      (add-text-properties
-- 
2.33.0

Re: Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

[Bastien]

Timothy <tecosaur@gmail.com> writes:

>> After you send it here, you can push to bugfix and merge the bugfix
>> branch into main.
>
> I’ll do exactly this shortly.

Thanks!

-- 
 Bastien

Re: Bug: Fontify error with markdown source block [9.4.4 (release_9.4.4 @ /usr/share/emacs/28.0.50/lisp/org/)]

[Ihor Radchenko]

Timothy <tecosaur@gmail.com> writes:

>> You can also add the X-Woof-Patch: applied header to your reply.

Applied.
Fixed.

For record, the patch has been applied, and this bug report is now
considered fixed.

-- 
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92