emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Re: Bug: org-src-font-lock-fontify-block should be wrapped with save-match-data [9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation! /mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and /mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/)
@ 2021-08-25  4:05 Tobias Zawada
  0 siblings, 0 replies; 6+ messages in thread
From: Tobias Zawada @ 2021-08-25  4:05 UTC (permalink / raw)
  To: manikulin; +Cc: emacs-orgmode


Dear Timothy and Maxim,

the bug was caused by an advice. I am really sorry for the trouble I 
inflicted on you.
Next time I will test more carefully.

Btw, I corrected the real problem already in 
https://github.com/TobiasZawada/org-src-emph.

Regards,
Tobias Zawada


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

* Re: Bug: org-src-font-lock-fontify-block should be wrapped with save-match-data [9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation! /mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and /mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/)
  2021-08-25 11:56     ` Maxim Nikulin
@ 2021-08-31 11:28       ` Timothy
  0 siblings, 0 replies; 6+ messages in thread
From: Timothy @ 2021-08-31 11:28 UTC (permalink / raw)
  To: Maxim Nikulin; +Cc: emacs-orgmode

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

Hi Maxim,

> Bug: org-src-font-lock-fontify-block should be wrapped with save-match-data
> [9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation!
> /mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and
> /mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/)
>
> Notice “mixed installation!”. Often it is source of obscure errors.
> <https://orgmode.org/worg/org-faq.html#mixed-install>
>
> Maybe link to FAQ item should be added to bug report template when mixed
> installation is detected. Since mixed installation can be detected, I would
> consider issuing a message during loading of Org mode.

Ah, thanks for highlighting this. It sounds like “mixed installation” is a red
flag which should prompt a request to reproduce the issue with an unmodified
Org.

All the best,
Timothy

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

* Re: Bug: org-src-font-lock-fontify-block should be wrapped with save-match-data [9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation! /mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and /mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/)
  2021-08-24 16:57   ` Maxim Nikulin
@ 2021-08-25 11:56     ` Maxim Nikulin
  2021-08-31 11:28       ` Timothy
  0 siblings, 1 reply; 6+ messages in thread
From: Maxim Nikulin @ 2021-08-25 11:56 UTC (permalink / raw)
  To: emacs-orgmode

In my previous message I forgot to mention the subject of the initial 
bug report:

Bug: org-src-font-lock-fontify-block should be wrapped with 
save-match-data [9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation! 
/mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and 
/mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/)

Notice "mixed installation!". Often it is source of obscure errors.
https://orgmode.org/worg/org-faq.html#mixed-install

Maybe link to FAQ item should be added to bug report template when mixed 
installation is detected. Since mixed installation can be detected, I 
would consider issuing a message during loading of Org mode.

On 24/08/2021 23:57, Maxim Nikulin wrote:
> On 23/08/2021 14:45, Timothy wrote:
>>
>> Thanks for your efforts. I have prepared a patch accordingly that wraps
>> org-src-font-lock-fontify-block’s body with save-match-data (attached).
 >
> The following question may be dumb since I am not familiar how font lock 
> works in Emacs. Is it necessary to wrap whole function? I do not see 
> explicit operation with regexps. The only suspecting line is
> 
>    (org-font-lock-ensure)

I think, now it is irrelevant, so just for completeness: initialization 
of major mode might affect match data as well

     (unless (eq major-mode lang-mode) (funcall lang-mode))



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

* Re: Bug: org-src-font-lock-fontify-block should be wrapped with save-match-data [9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation! /mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and /mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/)
  2021-08-23  7:45 ` Timothy
@ 2021-08-24 16:57   ` Maxim Nikulin
  2021-08-25 11:56     ` Maxim Nikulin
  0 siblings, 1 reply; 6+ messages in thread
From: Maxim Nikulin @ 2021-08-24 16:57 UTC (permalink / raw)
  To: emacs-orgmode

On 23/08/2021 14:45, Timothy wrote:
> 
> Thanks for your efforts. I have prepared a patch accordingly that wraps
> org-src-font-lock-fontify-block’s body with save-match-data (attached).
> 
> If I don’t hear anything bad about it in the next few days, I’ll push it :)
> Please let me know if my commit message agrees with your understanding of the
> issue.

Timothy, are you able to reproduce the issue with performance? Due to 
the thread
https://orgmode.org/list/CAFhsWEgAb_im1WpXp3xsfFxcoahKyycM4GaqRin0SUXxD0gMzg@mail.gmail.com/ 
"Large source block causes org-mode to be unusable" I assume that it 
could be quite severe. On the other hand I tried to insert an .el file 
into org-guide.org and I have not noticed slow down. Actually my 
curiosity is caused by other performance issues, so I am interested if 
the effect may be broader.

The following question may be dumb since I am not familiar how font lock 
works in Emacs. Is it necessary to wrap whole function? I do not see 
explicit operation with regexps. The only suspecting line is

   (org-font-lock-ensure)

I am not sure whether the body of `org-src-font-lock-fontify-block' 
should be wrapped or its call site.

Feel free to disregard my questions since fontification is far aside 
from my experience.




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

* Re: Bug: org-src-font-lock-fontify-block should be wrapped with save-match-data [9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation! /mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and /mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/)
  2021-08-20 10:07 Tobias Zawada
@ 2021-08-23  7:45 ` Timothy
  2021-08-24 16:57   ` Maxim Nikulin
  0 siblings, 1 reply; 6+ messages in thread
From: Timothy @ 2021-08-23  7:45 UTC (permalink / raw)
  To: Tobias Zawada; +Cc: emacs-orgmode

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

Hi Tobias,

Thanks for your efforts. I have prepared a patch accordingly that wraps
org-src-font-lock-fontify-block’s body with save-match-data (attached).

If I don’t hear anything bad about it in the next few days, I’ll push it :)
Please let me know if my commit message agrees with your understanding of the
issue.

All the best,
Timothy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-src-Save-match-data-when-fontifying-src-block.patch --]
[-- Type: text/x-patch, Size: 4409 bytes --]

From c435fd41428b6eb4f9f7971e73ca0a422006461b Mon Sep 17 00:00:00 2001
From: TEC <tec@tecosaur.com>
Date: Mon, 23 Aug 2021 15:09:24 +0800
Subject: [PATCH] org-src: Save match data when fontifying src block

* lisp/org-src.el (org-src-font-lock-fontify-block): Since
`org-src-font-lock-fontify-block' modifies match data during
fontification, when called in `org-fontify-meta-lines-and-blocks-1' by
`font-lock-fontify-region' the text property font-lock-multiline is
applied to text from the beginning to the last match.  Since there is a
difference in buffer sizes, the match data is invalid and problematic.
This issue can drastically slow down editing operations in large source
blocks.  This can be avoided simply by wrapping `save-match-data' around
`org-src-font-lock-fontify-block'.

Reported by: "Tobias Zawada" <i_inbox@tn-home.de>
<https://lists.gnu.org/archive/html/emacs-orgmode/2021-08/msg00307.html>
---
 lisp/org-src.el | 69 +++++++++++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/lisp/org-src.el b/lisp/org-src.el
index 4698c6dd2..ce33e1f54 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -586,40 +586,41 @@ (defun org-src-font-lock-fontify-block (lang start end)
   "Fontify code block.
 This function is called by emacs automatic fontification, as long
 as `org-src-fontify-natively' is non-nil."
-  (let ((lang-mode (org-src-get-lang-mode lang)))
-    (when (fboundp lang-mode)
-      (let ((string (buffer-substring-no-properties start end))
-	    (modified (buffer-modified-p))
-	    (org-buffer (current-buffer)))
-	(remove-text-properties start end '(face nil))
-	(with-current-buffer
-	    (get-buffer-create
-	     (format " *org-src-fontification:%s*" lang-mode))
-	  (let ((inhibit-modification-hooks nil))
-	    (erase-buffer)
-	    ;; Add string and a final space to ensure property change.
-	    (insert string " "))
-	  (unless (eq major-mode lang-mode) (funcall lang-mode))
-	  (org-font-lock-ensure)
-	  (let ((pos (point-min)) next)
-	    (while (setq next (next-property-change pos))
-	      ;; Handle additional properties from font-lock, so as to
-	      ;; preserve, e.g., composition.
-	      (dolist (prop (cons 'face font-lock-extra-managed-props))
-		(let ((new-prop (get-text-property pos prop)))
-		  (put-text-property
-		   (+ start (1- pos)) (1- (+ start next)) prop new-prop
-		   org-buffer)))
-	      (setq pos next))))
-	;; Add Org faces.
-	(let ((src-face (nth 1 (assoc-string lang org-src-block-faces t))))
-          (when (or (facep src-face) (listp src-face))
-            (font-lock-append-text-property start end 'face src-face))
-	  (font-lock-append-text-property start end 'face 'org-block))
-	(add-text-properties
-	 start end
-	 '(font-lock-fontified t fontified t font-lock-multiline t))
-	(set-buffer-modified-p modified)))))
+  (save-match-data
+    (let ((lang-mode (org-src-get-lang-mode lang)))
+      (when (fboundp lang-mode)
+	(let ((string (buffer-substring-no-properties start end))
+	      (modified (buffer-modified-p))
+	      (org-buffer (current-buffer)))
+	  (remove-text-properties start end '(face nil))
+	  (with-current-buffer
+	      (get-buffer-create
+	       (format " *org-src-fontification:%s*" lang-mode))
+	    (let ((inhibit-modification-hooks nil))
+	      (erase-buffer)
+	      ;; Add string and a final space to ensure property change.
+	      (insert string " "))
+	    (unless (eq major-mode lang-mode) (funcall lang-mode))
+	    (org-font-lock-ensure)
+	    (let ((pos (point-min)) next)
+	      (while (setq next (next-property-change pos))
+		;; Handle additional properties from font-lock, so as to
+		;; preserve, e.g., composition.
+		(dolist (prop (cons 'face font-lock-extra-managed-props))
+		  (let ((new-prop (get-text-property pos prop)))
+		    (put-text-property
+		     (+ start (1- pos)) (1- (+ start next)) prop new-prop
+		     org-buffer)))
+		(setq pos next))))
+	  ;; Add Org faces.
+	  (let ((src-face (nth 1 (assoc-string lang org-src-block-faces t))))
+	    (when (or (facep src-face) (listp src-face))
+	      (font-lock-append-text-property start end 'face src-face))
+	    (font-lock-append-text-property start end 'face 'org-block))
+	  (add-text-properties
+	   start end
+	   '(font-lock-fontified t fontified t font-lock-multiline t))
+	  (set-buffer-modified-p modified))))))
 
 \f
 ;;; Escape contents
-- 
2.32.0


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

* Bug: org-src-font-lock-fontify-block should be wrapped with save-match-data [9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation! /mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and /mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/)
@ 2021-08-20 10:07 Tobias Zawada
  2021-08-23  7:45 ` Timothy
  0 siblings, 1 reply; 6+ messages in thread
From: Tobias Zawada @ 2021-08-20 10:07 UTC (permalink / raw)
  To: emacs-orgmode

~org-src-font-lock-fontify-block~ modifies ~match-data~ through the
fontification of the temporary source buffer. But
~org-src-font-lock-fontify-block~ is also called in
~org-fontify-meta-lines-and-blocks-1~ by
~font-lock-fontify-region~. There it puts the text property
~font-lock-multiline~ on some text from the beginning up to the end of
the last match in the Org buffer. Since the source buffer is smaller than the Org buffer
~match-beginning~ is smaller than it should be.

This can slow down editing operations in org-mode with large source blocks to an extent to which
org-mode becomes unusable.

An easy workaround is:

#+begin_src emacs-lisp
(defun org+-with-save-match-data (fun &rest args)
  "Run FUN with ARGS but save `match-data'."
  (save-match-data
    (apply fun args)))

(advice-add 'org-src-font-lock-fontify-block :around #'org+-with-save-match-data)
#+end_src

Emacs  : GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
 of 2020-09-19
Package: Org mode version 9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation! /mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and /mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/)


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

end of thread, other threads:[~2021-08-31 11:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-25  4:05 Bug: org-src-font-lock-fontify-block should be wrapped with save-match-data [9.3.7 (9.3.7-4-gba6ca7-elpaplus @ mixed installation! /mnt/c/Users/toz/Weiterbildung/Soft/Emacs/ and /mnt/c/Users/toz/.emacs.d/elpa/org-plus-contrib-20200615/) Tobias Zawada
  -- strict thread matches above, loose matches on Subject: below --
2021-08-20 10:07 Tobias Zawada
2021-08-23  7:45 ` Timothy
2021-08-24 16:57   ` Maxim Nikulin
2021-08-25 11:56     ` Maxim Nikulin
2021-08-31 11:28       ` Timothy

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs/org-mode.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).