emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* 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; 9+ 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] 9+ 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 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
@ 2021-08-23  7:45 ` Timothy
  2021-08-24 16:57   ` Maxim Nikulin
  0 siblings, 1 reply; 9+ 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] 9+ 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  8:07     ` What happened to ./contrib? Martin Steffen
  2021-08-25 11:56     ` 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/) Maxim Nikulin
  0 siblings, 2 replies; 9+ 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] 9+ messages in thread

* What happened to ./contrib?
  2021-08-24 16:57   ` Maxim Nikulin
@ 2021-08-25  8:07     ` Martin Steffen
  2021-08-25  8:44       ` Tim Cross
  2021-08-25 11:47       ` Maxim Nikulin
  2021-08-25 11:56     ` 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/) Maxim Nikulin
  1 sibling, 2 replies; 9+ messages in thread
From: Martin Steffen @ 2021-08-25  8:07 UTC (permalink / raw)
  To: emacs-orgmode



Hi,

I use org-mode, having cloned the git-version, pulling freshest versions
from time to time. Without a definite schedule when I do that I don't
know exactly when the change occured that I am asking here about.

Anyway, my  Org mode version is 9.4.6 (release_9.4.6-598-g604bfd @


Now: today I notices, some org-mode packages/files are missing. As said,
I don't know since when that was the case. In particular, emacs
initialization could no longer do

    (require 'ox-extra)

since the file is gone. I looked around that not just file is no longer
part of the clone, but the hole

     contrib/lisp

directory (which is where that one was located) is gone.

Is there a reason for that, resp. is the functionality of ox-extra been
moved somewhere else?

Note that the documentation

  https://orgmode.org/worg/org-contrib/

(still) mentions currently a directory contrib, but in my latest pulled
version, it's gone (see also https://github.com/bzg/org-mode).

If course I can roll-back to an earlier revision or download one to get
back for instance ox-extra (which has a feature I rely on), but perhaps
there's something wrong.

Thanks, Martin


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

* Re: What happened to ./contrib?
  2021-08-25  8:07     ` What happened to ./contrib? Martin Steffen
@ 2021-08-25  8:44       ` Tim Cross
  2021-08-25 11:47       ` Maxim Nikulin
  1 sibling, 0 replies; 9+ messages in thread
From: Tim Cross @ 2021-08-25  8:44 UTC (permalink / raw)
  To: emacs-orgmode


The contrib directory has been broken out into its own git repository at
https://git.sr.ht/~bzg/org-contrib and the main org repository is moving
to be hosted on savannah.gnu.org (I think it is mirrored there already).

This is all in preparation for having the contrib extensions available
as an ELPA archive in the nonGNU ELPA repository and core org being
available in the GNU ELPA repository.

Things are in a transition stage at present, so there is still a bit of
documentation updates/fixes needed to be completed. Updates are being
sent to this list.

HTH

Martin Steffen <msteffen@ifi.uio.no> writes:

> Hi,
>
> I use org-mode, having cloned the git-version, pulling freshest versions
> from time to time. Without a definite schedule when I do that I don't
> know exactly when the change occured that I am asking here about.
>
> Anyway, my  Org mode version is 9.4.6 (release_9.4.6-598-g604bfd @
>
>
> Now: today I notices, some org-mode packages/files are missing. As said,
> I don't know since when that was the case. In particular, emacs
> initialization could no longer do
>
>     (require 'ox-extra)
>
> since the file is gone. I looked around that not just file is no longer
> part of the clone, but the hole
>
>      contrib/lisp
>
> directory (which is where that one was located) is gone.
>
> Is there a reason for that, resp. is the functionality of ox-extra been
> moved somewhere else?
>
> Note that the documentation
>
>   https://orgmode.org/worg/org-contrib/
>
> (still) mentions currently a directory contrib, but in my latest pulled
> version, it's gone (see also https://github.com/bzg/org-mode).
>
> If course I can roll-back to an earlier revision or download one to get
> back for instance ox-extra (which has a feature I rely on), but perhaps
> there's something wrong.
>
> Thanks, Martin



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

* Re: What happened to ./contrib?
  2021-08-25  8:07     ` What happened to ./contrib? Martin Steffen
  2021-08-25  8:44       ` Tim Cross
@ 2021-08-25 11:47       ` Maxim Nikulin
  1 sibling, 0 replies; 9+ messages in thread
From: Maxim Nikulin @ 2021-08-25 11:47 UTC (permalink / raw)
  To: Martin Steffen, emacs-orgmode

On 25/08/2021 15:07, Martin Steffen wrote:
> 
> Note that the documentation
> 
>    https://orgmode.org/worg/org-contrib/
> 
> (still) mentions currently a directory contrib, but in my latest pulled
> version, it's gone (see also https://github.com/bzg/org-mode).

There is a patch that should fix links to raw files on worg:
https://orgmode.org/list/sc4k26$uf6$1@ciao.gmane.io

See https://orgmode.org/list/874kf4fanb.fsf@gnu.org/ or
https://lists.gnu.org/archive/html/emacs-orgmode/2021-05/msg00769.html
for the announce of org-contrib package:

Bastien writes:
> You can now install org-contrib as a new NonGNU ELPA package:
> https://elpa.nongnu.org/nongnu/

P.S. Please, do not use "reply" to ask new question since it merges 
unrelated discussions into single thread. It is better to compose new 
message.


^ permalink raw reply	[flat|nested] 9+ 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  8:07     ` What happened to ./contrib? Martin Steffen
@ 2021-08-25 11:56     ` Maxim Nikulin
  2021-08-31 11:28       ` Timothy
  1 sibling, 1 reply; 9+ 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] 9+ 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     ` 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/) Maxim Nikulin
@ 2021-08-31 11:28       ` Timothy
  0 siblings, 0 replies; 9+ 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] 9+ 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  4:05 Tobias Zawada
  0 siblings, 0 replies; 9+ 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] 9+ messages in thread

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

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-20 10:07 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
2021-08-23  7:45 ` Timothy
2021-08-24 16:57   ` Maxim Nikulin
2021-08-25  8:07     ` What happened to ./contrib? Martin Steffen
2021-08-25  8:44       ` Tim Cross
2021-08-25 11:47       ` Maxim Nikulin
2021-08-25 11:56     ` 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/) Maxim Nikulin
2021-08-31 11:28       ` Timothy
2021-08-25  4:05 Tobias Zawada

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 NNTP newsgroup(s).