emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@gmail.com>
To: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Cc: emacs-orgmode@gnu.org
Subject: Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers
Date: Sun, 17 May 2020 23:00:10 +0800	[thread overview]
Message-ID: <87tv0efvyd.fsf@localhost> (raw)
In-Reply-To: <87pnbby49m.fsf@nicolasgoaziou.fr>

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

Hi,

[All the changes below are relative to commit ed0e75d24. Later commits
make it hard to distinguish between hidden headlines and drawers. I will
need to figure out a way to merge this branch with master. It does not
seem to be trivial.]

I have finished a seemingly stable implementation of handling changes
inside drawer and block elements. For now, I did not bother with
'modification-hooks and 'insert-in-font/behind-hooks, but simply used
before/after-change-functions.

The basic idea is saving parsed org-elements before the modification
(with :begin and :end replaced by markers) and comparing them with the 
versions of the same elements after the modification.
Any valid org element can be examined in such way by an arbitrary
function (see org-track-modification-elements) [1].

For now, I have implemented tracking changes in all the drawer and block
elements. If the contents of an element is changed and the element is
hidden, the contents remains hidden unless the change was done with
self-insert-command. If the begin/end line of the element was changed in
the way that the element changes the type or extends/shrinks, the
element contents is revealed. To illustrate:

Case #1 (the element content is hidden):

:PROPERTIES:
:ID:       279e797c-f4a7-47bb-80f6-e72ac6f3ec55
:END:

is changed to

:ROPERTIES:
:ID:       279e797c-f4a7-47bb-80f6-e72ac6f3ec55
:END:

Text is revealed, because we have drawer in place of property-drawer

Case #2 (the element content is hidden):

:ROPERTIES:
:ID:       279e797c-f4a7-47bb-80f6-e72ac6f3ec55
:END:

is changed to

:OPERTIES:
:ID:       279e797c-f4a7-47bb-80f6-e72ac6f3ec55
:END:

The text remains hidden since it is still a drawer.

Case #3: (the element content is hidden):

:FOO:
bar
tmp
:END:

is changed to

:FOO:
bar
:END:
tmp
:END:

The text is revealed because the drawer contents shrank.

Case #4: (the element content is hidden in both the drawers):

:FOO:
bar
tmp
:END:
:BAR:
jjd
:END:

is changed to

:FOO:
bar
tmp
:BAR:
jjd
:END:

The text is revealed in both the drawers because the drawers are merged
into a new drawer.

> However, I think we can do better than that, and also fix the glitches
> from overlays. Here are two of them. Write the following drawer:
>
>   :FOO:
>   bar
>   :END:
>
> fold it and delete the ":f". The overlay is still there, and you cannot
> remove it with TAB any more. Or, with the same initial drawer, from
> beginning of buffer, evaluate:
>
>   (progn (re-search-forward ":END:") (replace-match ""))
>
> This is no longer a drawer: you just removed its closing line. Yet, the
> overlay is still there, and TAB is ineffective.

I think the above examples cover what you described.

Case #5 (the element content is hidden, point at <!>):

:FOO:<!>
bar
tmp
:END:

is changed (via self-insert-command) to

:FOO:a<!>
bar
tmp
:END:

The text is revealed.

This last case sounds logical and might potentially replace
org-catch-invisible-edits.

------------------------------------------------------------------------

Some potential issues with the implementation:

1. org--after-element-change-function can called many times even for
trivial operations. For example (insert "\n" ":TEST:") seems to call it
two times already. This has two implications: (1) potential performance
degradation; (2) org-element library may not be able to parse the
changed element because its intermediate modified state may not match
the element syntax. Specifically, inserting new property into
:PROPERTIES: drawer inserts a newline at some point, which makes
org-element-at-point think that it is not a 'property-drawer, but just
'drawer.

For (1), I did not really do any workaround for now. One potential way
is making use of combine-after-change-calls (info:elisp#Change Hooks).
At least, within org source code. 

For (2), I have introduced org--property-drawer-modified-re to override
org-property-drawer-re in relevant *-change-function. This seems to work
for property drawers. However, I am not sure if similar problem may
happen in some border cases with ordinary drawers or blocks. 

2. I have noticed that results of org-element-at-point and
org-element-parse-buffer are not always consistent.
In my tests, they returned different number of empty lines after drawers
(:post-blank and :end properties). I am not sure here if I did something
wrong in the code or if it is a real issue in org-element.

For now, I simply called org-element-at-point with point at :begin
property of all the elements returned by org-element-parse buffer to
make things consistent. This indeed introduced overhead, but I do not
see other way to solve the inconsistency.

3. This implementation did not directly solve the previously observed
issue with two ellipsis displayed in folded drawer after adding hidden
text inside:

:PROPERTY: ...  -->  :PROPERTY: ...  ...

For now, I just did

(org-hide-drawer-toggle 'off)
(org-hide-drawer-toggle 'hide)

to hide the second ellipsis, but I still don't understand why it is
happening. Is it some Emacs bug? I am not sure.

4. For some reason, before/after-change-functions do not seem to trigger
when adding note after todo state change. 

------------------------------------------------------------------------

Further plans:

1. Investigate the issue with log notes.
2. Try to change headings to use text properties as well.

The current version of the patch (relative to commit ed0e75d24) is
attached. 

------------------------------------------------------------------------

P.S. I have noticed an issue with hidden text on master (9bc0cc7fb) with
my personal config:

For the following .org file:

* TODO b
:PROPERTIES:
:CREATED:  [2020-05-17 Sun 22:37]
:END:

folded to

* TODO b...

Changing todo to DONE will be shown as

* DONE b
CLOSED: [2020-05-17 Sun 22:54]...:LOGBOOK:...

------------------------------------------------------------------------

[1] If one wants to track changes in two elements types, where one is
always inside the other, it is not possible now.

Best,
Ihor


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

diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index a02f713ca..4b0e23f6a 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -682,7 +682,7 @@ When NEXT is non-nil, check the next line instead."
 
 
 \f
-;;; Overlays
+;;; Overlays and text properties
 
 (defun org-overlay-display (ovl text &optional face evap)
   "Make overlay OVL display TEXT with face FACE."
@@ -705,18 +705,44 @@ If DELETE is non-nil, delete all those overlays."
 	    (delete (delete-overlay ov))
 	    (t (push ov found))))))
 
+(defun org--find-text-property-region (pos prop)
+  "Find a region containing PROP text property around point POS."
+  (let* ((beg (and (get-text-property pos prop) pos))
+	 (end beg))
+    (when beg
+      ;; when beg is the first point in the region, `previous-single-property-change'
+      ;; will return nil.
+      (setq beg (or (previous-single-property-change pos prop)
+		    beg))
+      ;; when end is the last point in the region, `next-single-property-change'
+      ;; will return nil.
+      (setq end (or (next-single-property-change pos prop)
+		    end))
+      (unless (= beg end) ; this should not happen
+        (cons beg end)))))
+
 (defun org-flag-region (from to flag spec)
   "Hide or show lines from FROM to TO, according to FLAG.
 SPEC is the invisibility spec, as a symbol."
-  (remove-overlays from to 'invisible spec)
-  ;; Use `front-advance' since text right before to the beginning of
-  ;; the overlay belongs to the visible line than to the contents.
-  (when flag
-    (let ((o (make-overlay from to nil 'front-advance)))
-      (overlay-put o 'evaporate t)
-      (overlay-put o 'invisible spec)
-      (overlay-put o 'isearch-open-invisible #'delete-overlay))))
-
+  (pcase spec
+    ('outline
+     (remove-overlays from to 'invisible spec)
+     ;; Use `front-advance' since text right before to the beginning of
+     ;; the overlay belongs to the visible line than to the contents.
+     (when flag
+       (let ((o (make-overlay from to nil 'front-advance)))
+	 (overlay-put o 'evaporate t)
+	 (overlay-put o 'invisible spec)
+	 (overlay-put o 'isearch-open-invisible #'delete-overlay))))
+    (_
+     ;; Use text properties instead of overlays for speed.
+     ;; Overlays are too slow (Emacs Bug#35453).
+     (with-silent-modifications
+       (remove-text-properties from to '(invisible nil))
+       (when flag
+	 (put-text-property from to 'rear-non-sticky nil)
+	 (put-text-property from to 'front-sticky t)
+	 (put-text-property from to 'invisible spec))))))
 
 \f
 ;;; Regexp matching
diff --git a/lisp/org.el b/lisp/org.el
index 96e7384f3..1bf90edae 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -114,6 +114,7 @@ Stars are put in group 1 and the trimmed body in group 2.")
 (declare-function cdlatex-math-symbol "ext:cdlatex")
 (declare-function Info-goto-node "info" (nodename &optional fork strict-case))
 (declare-function isearch-no-upper-case-p "isearch" (string regexp-flag))
+(declare-function isearch-filter-visible "isearch" (beg end))
 (declare-function org-add-archive-files "org-archive" (files))
 (declare-function org-agenda-entry-get-agenda-timestamp "org-agenda" (pom))
 (declare-function org-agenda-list "org-agenda" (&optional arg start-day span with-hour))
@@ -192,6 +193,9 @@ Stars are put in group 1 and the trimmed body in group 2.")
 
 (defvar ffap-url-regexp)
 (defvar org-element-paragraph-separate)
+(defvar org-element-all-objects)
+(defvar org-element-all-elements)
+(defvar org-element-greater-elements)
 (defvar org-indent-indentation-per-level)
 (defvar org-radio-target-regexp)
 (defvar org-target-link-regexp)
@@ -4737,6 +4741,153 @@ This is for getting out of special buffers like capture.")
 (defun org-before-change-function (_beg _end)
   "Every change indicates that a table might need an update."
   (setq org-table-may-need-update t))
+
+(defvar-local org--modified-elements nil
+  "List of unmodified versions of recently modified elements.
+
+The :begin and :end element properties contain markers instead of positions.")
+
+(defvar org--property-drawer-modified-re (concat (replace-regexp-in-string "\\$$" "\n" org-property-start-re)
+					      "\\(?:.*\n\\)*?"
+                                              (replace-regexp-in-string "^\\^" "" org-property-end-re))
+  "Matches entire property drawer, including its state during modification.
+
+This should be different from `org-property-drawer-re' because
+property drawer may contain empty or incomplete lines in the middle of
+modification.")
+
+(defun org--drawer-or-block-change-function (el)
+  "Update visibility of changed drawer/block EL.
+
+If text was added to hidden drawer/block,
+make sure that the text is also hidden, unless
+the change was done by `self-insert-command'.
+If the modification destroyed the drawer/block,
+reveal the hidden text in former drawer/block."
+  (save-match-data
+    (save-excursion
+      (save-restriction
+	(goto-char (org-element-property :begin el))
+	(let* ((newel (org-element-at-point))
+               (spec (if (string-match-p "block" (symbol-name (org-element-type el)))
+			 'org-hide-block
+                       (if (string-match-p "drawer" (symbol-name (org-element-type el)))
+			   'org-hide-drawer
+			 t))))
+	  (if (and (equal (org-element-type el) (org-element-type newel))
+		   (equal (marker-position (org-element-property :begin el))
+			  (org-element-property :begin newel))
+		   (equal (marker-position (org-element-property :end el))
+			  (org-element-property :end newel)))
+	      (when (text-property-any (marker-position (org-element-property :begin el))
+				       (marker-position (org-element-property :end el))
+                                       'invisible spec)
+		(if (memq this-command '(self-insert-command))
+		    ;; reveal if change was made by typing
+		    (org-hide-drawer-toggle 'off)
+		  ;; re-hide the inserted text
+		  ;; FIXME: opening the drawer before hiding should not be needed here
+		  (org-hide-drawer-toggle 'off) ; this is needed to avoid showing double ellipsis
+		  (org-hide-drawer-toggle 'hide)))
+            ;; The element was destroyed. Reveal everything.
+            (org-flag-region (marker-position (org-element-property :begin el))
+			  (marker-position (org-element-property :end el))
+			  nil spec)
+            (org-flag-region (org-element-property :begin newel)
+			  (org-element-property :end newel)
+			  nil spec)))))))
+
+(defvar org-track-modification-elements (list (cons 'center-block #'org--drawer-or-block-change-function)
+					      (cons 'drawer #'org--drawer-or-block-change-function)
+					      (cons 'dynamic-block #'org--drawer-or-block-change-function)
+					      (cons 'property-drawer #'org--drawer-or-block-change-function)
+					      (cons 'quote-block #'org--drawer-or-block-change-function)
+					      (cons 'special-block #'org--drawer-or-block-change-function))
+  "Alist of elements to be tracked for modifications.
+Each element of the alist is a cons of an element from
+`org-element-all-elements' and the function used to handle the
+modification.
+The function must accept a single argument - parsed element before
+modificatin with :begin and :end properties containing markers.")
+
+(defun org--find-elements-in-region (beg end elements &optional include-partial)
+  "Find all elements from ELEMENTS list in region BEG . END.
+All the listed elements must be resolvable by `org-element-at-point'.
+Include elements if they are partially inside region when INCLUDE-PARTIAL is non-nil."
+  (when include-partial
+    (org-with-point-at beg
+      (when-let ((new-beg (org-element-property :begin
+					     (org-element-lineage (org-element-at-point)
+							       elements
+							       'with-self))))
+	(setq beg new-beg))
+      (when (memq 'headline elements)
+	(when-let ((new-beg (ignore-error user-error (org-back-to-heading 'include-invisible))))
+          (setq beg new-beg))))
+    (org-with-point-at end
+      (when-let ((new-end (org-element-property :end
+					     (org-element-lineage (org-element-at-point)
+							       elements
+							       'with-self))))
+	(setq end new-end))
+      (when (memq 'headline elements)
+	(when-let ((new-end (org-with-limited-levels (outline-next-heading))))
+          (setq end (1- new-end))))))
+  (save-excursion
+    (save-restriction
+      (narrow-to-region beg end)
+      (let (has-object has-element has-greater-element granularity)
+	(dolist (el elements)
+	  (when (memq el org-element-all-objects) (setq has-object t))
+	  (when (memq el org-element-all-elements) (setq has-element t))
+	  (when (memq el org-element-greater-elements) (setq has-greater-element t)))
+	(if has-object
+	    (setq granularity 'object)
+	  (if has-greater-element
+              (setq granularity 'greater-element)
+            (if has-element
+		(setq granularity 'element)
+              (setq granularity 'headline))))
+	(org-element-map (org-element-parse-buffer granularity) elements #'identity)))))
+
+(defun org--before-element-change-function (beg end)
+  "Register upcoming element modifications in `org--modified-elements' for all elements interesting with BEG END."
+  (let ((org-property-drawer-re org--property-drawer-modified-re))
+    (save-match-data
+      (save-excursion
+	(save-restriction
+	  (dolist (el (org--find-elements-in-region beg
+						 end
+						 (mapcar #'car org-track-modification-elements)
+						 'include-partial))
+            ;; `org-element-at-point' is not consistent with results
+	    ;; of `org-element-parse-buffer' for :post-blank and :end
+            ;; Using `org-element-at-point to keep consistent
+            ;; parse results with `org--after-element-change-function'
+	    (let* ((el (org-with-point-at (org-element-property :begin el)
+			 (org-element-at-point)))
+		   (beg-marker (copy-marker (org-element-property :begin el) 't))
+		   (end-marker (copy-marker (org-element-property :end el) 't)))
+	      (when (and (marker-position beg-marker) (marker-position end-marker))
+		(org-element-put-property el :begin beg-marker)
+		(org-element-put-property el :end end-marker)
+		(add-to-list 'org--modified-elements el)))))))))
+
+;; FIXME: this function may be called many times during routine modifications
+;; The normal way to avoid this is `combine-after-change-calls' - not
+;; the case in most org primitives.
+(defun org--after-element-change-function (&rest _)
+  "Handle changed elements from `org--modified-elements'."
+  (let ((org-property-drawer-re org--property-drawer-modified-re))
+    (dolist (el org--modified-elements)
+      (save-match-data
+	(save-excursion
+          (save-restriction
+	    (let* ((type (org-element-type el))
+		   (change-func (alist-get type org-track-modification-elements)))
+	      (funcall (symbol-function change-func) el)))))))
+  (setq org--modified-elements nil))
+
 (defvar org-mode-map)
 (defvar org-inhibit-startup-visibility-stuff nil) ; Dynamically-scoped param.
 (defvar org-agenda-keep-modes nil)      ; Dynamically-scoped param.
@@ -4818,6 +4969,9 @@ The following commands are available:
   ;; Activate before-change-function
   (setq-local org-table-may-need-update t)
   (add-hook 'before-change-functions 'org-before-change-function nil 'local)
+  (add-hook 'before-change-functions 'org--before-element-change-function nil 'local)
+  ;; Activate after-change-function
+  (add-hook 'after-change-functions 'org--after-element-change-function nil 'local)
   ;; Check for running clock before killing a buffer
   (add-hook 'kill-buffer-hook 'org-check-running-clock nil 'local)
   ;; Initialize macros templates.
@@ -4869,6 +5023,10 @@ The following commands are available:
   (setq-local outline-isearch-open-invisible-function
 	      (lambda (&rest _) (org-show-context 'isearch)))
 
+  ;; Make isearch search in blocks hidden via text properties
+  (setq-local isearch-filter-predicate #'org--isearch-filter-predicate)
+  (add-hook 'isearch-mode-end-hook #'org--clear-isearch-overlays nil 'local)
+
   ;; Setup the pcomplete hooks
   (setq-local pcomplete-command-completion-function #'org-pcomplete-initial)
   (setq-local pcomplete-command-name-function #'org-command-at-point)
@@ -5859,9 +6017,26 @@ If TAG is a number, get the corresponding match group."
 	 (inhibit-modification-hooks t)
 	 deactivate-mark buffer-file-name buffer-file-truename)
     (decompose-region beg end)
+    ;; do not remove invisible text properties specified by
+    ;; 'org-hide-block and 'org-hide-drawer (but remove  'org-link)
+    ;; this is needed to keep the drawers and blocks hidden unless
+    ;; they are toggled by user
+    ;; Note: The below may be too specific and create troubles
+    ;; if more invisibility specs are added to org in future
+    (let ((pos beg)
+	  next spec)
+      (while (< pos end)
+	(setq next (next-single-property-change pos 'invisible nil end)
+              spec (get-text-property pos 'invisible))
+        (unless (memq spec (list 'org-hide-block
+				 'org-hide-drawer))
+          (remove-text-properties pos next '(invisible t)))
+        (setq pos next)))
     (remove-text-properties beg end
 			    '(mouse-face t keymap t org-linked-text t
-					 invisible t intangible t
+					 ;; Do not remove all invisible during fontification
+					 ;; invisible t
+                                         intangible t
 					 org-emphasis t))
     (org-remove-font-lock-display-properties beg end)))
 
@@ -6666,8 +6841,13 @@ information."
     ;; expose it.
     (dolist (o (overlays-at (point)))
       (when (memq (overlay-get o 'invisible)
-		  '(org-hide-block org-hide-drawer outline))
+		  '(outline))
 	(delete-overlay o)))
+    (when (memq (get-text-property (point) 'invisible)
+		'(org-hide-block org-hide-drawer))
+      (let ((spec (get-text-property (point) 'invisible))
+	    (region (org--find-text-property-region (point) 'invisible)))
+	(org-flag-region (car region) (cdr region) nil spec)))
     (unless (org-before-first-heading-p)
       (org-with-limited-levels
        (cl-case detail
@@ -20902,6 +21082,79 @@ Started from `gnus-info-find-node'."
           (t default-org-info-node))))))
 
 \f
+
+;;; Make isearch search in some text hidden via text propertoes
+
+(defvar org--isearch-overlays nil
+  "List of overlays temporarily created during isearch.
+This is used to allow searching in regions hidden via text properties.
+As for [2020-05-09 Sat], Isearch only has special handling of hidden overlays.
+Any text hidden via text properties is not revealed even if `search-invisible'
+is set to 't.")
+
+;; Not sure if it needs to be a user option
+;; One might want to reveal hidden text in, for example, hidden parts of the links.
+;; Currently, hidden text in links is never revealed by isearch.
+(defvar org-isearch-specs '(org-hide-block
+			 org-hide-drawer)
+  "List of text invisibility specs to be searched by isearch.
+By default ([2020-05-09 Sat]), isearch does not search in hidden text,
+which was made invisible using text properties. Isearch will be forced
+to search in hidden text with any of the listed 'invisible property value.")
+
+(defun org--create-isearch-overlays (beg end)
+  "Replace text property invisibility spec by overlays between BEG and END.
+All the regions with invisibility text property spec from
+`org-isearch-specs' will be changed to use overlays instead
+of text properties. The created overlays will be stored in
+`org--isearch-overlays'."
+  (let ((pos beg))
+    (while (< pos end)
+      (when-let* ((spec (get-text-property pos 'invisible))
+		  (spec (memq spec org-isearch-specs))
+		  (region (org--find-text-property-region pos 'invisible)))
+        ;; Changing text properties is considered buffer modification.
+        ;; We do not want it here.
+	(with-silent-modifications
+          ;; The overlay is modelled after `org-flag-region' [2020-05-09 Sat]
+          ;; overlay for 'outline blocks.
+          (let ((o (make-overlay (car region) (cdr region) nil 'front-advance)))
+	    (overlay-put o 'evaporate t)
+	    (overlay-put o 'invisible spec)
+            ;; `delete-overlay' here means that spec information will be lost
+            ;; for the region. The region will remain visible.
+	    (overlay-put o 'isearch-open-invisible #'delete-overlay)
+            (push o org--isearch-overlays))
+	  (remove-text-properties (car region) (cdr region) '(invisible nil))))
+      (setq pos (next-single-property-change pos 'invisible nil end)))))
+
+(defun org--isearch-filter-predicate (beg end)
+  "Return non-nil if text between BEG and END is deemed visible by Isearch.
+This function is intended to be used as `isearch-filter-predicate'.
+Unlike `isearch-filter-visible', make text with 'invisible text property
+value listed in `org-isearch-specs' visible to Isearch."
+  (org--create-isearch-overlays beg end) ;; trick isearch by creating overlays in place of invisible text
+  (isearch-filter-visible beg end))
+
+(defun org--clear-isearch-overlay (ov)
+  "Convert OV region back into using text properties."
+  (when-let ((spec (overlay-get ov 'invisible))) ;; ignore deleted overlays
+    ;; Changing text properties is considered buffer modification.
+    ;; We do not want it here.
+    (with-silent-modifications
+      (put-text-property (overlay-start ov) (overlay-end ov) 'invisible spec)))
+  (when (member ov isearch-opened-overlays)
+    (setq isearch-opened-overlays (delete ov isearch-opened-overlays)))
+  (delete-overlay ov))
+
+(defun org--clear-isearch-overlays ()
+  "Convert overlays from `org--isearch-overlays' back into using text properties."
+  (when org--isearch-overlays
+    (mapc #'org--clear-isearch-overlay org--isearch-overlays)
+    (setq org--isearch-overlays nil)))
+
+\f
+
 ;;; Finish up
 
 (add-hook 'org-mode-hook     ;remove overlays when changing major mode

[-- Attachment #3: Type: text/plain, Size: 4435 bytes --]



Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> This should be better:
>> https://gist.github.com/yantar92/e37c2830d3bb6db8678b14424286c930
>
> Thank you.
>
>> This might get tricky in the following case:
>>
>> :PROPERTIES:
>> :CREATED: [2020-04-13 Mon 22:31]
>> <region-beginning>
>> :SHOWFROMDATE: 2020-05-11
>> :ID:       e05e3b33-71ba-4bbc-abba-8a92c565ad34
>> :END:
>>
>> <many subtrees in between>
>>
>> :PROPERTIES:
>> :CREATED:  [2020-04-27 Mon 13:50]
>> <region-end>
>> :ID:       b2eef49f-1c5c-4ff1-8e10-80423c8d8532
>> :ATTACH_DIR_INHERIT: t
>> :END:
>>
>> If the text in the region is replaced by something else, <many subtrees
>> in between> should not be fully hidden. We cannot simply look at the
>> 'invisible property before and after the changed region. 
>
> Be careful: "replaced by something else" is ambiguous. "Replacing" is an
> unlikely change: you would need to do 
>
>   (setf (buffer-substring x y) "foo")
>
> We can safely assume this will not happen. If it does, we can accept the
> subsequent glitch.
>
> Anyway it is less confusing to think in terms of deletion and insertion.
> In the case above, you probably mean "the region is deleted then
> something else is inserted", or the other way. So there are two actions
> going on, i.e., `after-change-functions' are called twice.
>
> In particular the situation you foresee /cannot happen/ with an
> insertion. Text is inserted at a single point. Let's assume this is in
> the first drawer. Once inserted, both text before and after the new text
> were part of the same drawer. Insertion introduces other problems,
> though. More on this below.
>
> It is true the deletion can produce the situation above. But in this
> case, there is nothing to do, you just merged two drawers into a single
> one, which stays invisible. Problem solved.
>
> IOW, big changes like the one you describe are not an issue. I think the
> "check if previous and next parts match" trick gives us roughly the same
> functionality, and the same glitches, as overlays.
>
> However, I think we can do better than that, and also fix the glitches
> from overlays. Here are two of them. Write the following drawer:
>
>   :FOO:
>   bar
>   :END:
>
> fold it and delete the ":f". The overlay is still there, and you cannot
> remove it with TAB any more. Or, with the same initial drawer, from
> beginning of buffer, evaluate:
>
>   (progn (re-search-forward ":END:") (replace-match ""))
>
> This is no longer a drawer: you just removed its closing line. Yet, the
> overlay is still there, and TAB is ineffective.
>
> Here's an idea to develop that would make folding more robust, and still
> fast.
>
> Each syntactical element has a "sensitive part", a particular area that
> can change the nature of the element when it is altered. Luckily drawers
> (and blocks) are sturdy. For a drawer, there are three things to check:
>
> 1. the opening line must match org-drawer-regexp
> 2. the closing line must match org-property-end-re (case ignored)
> 3. between those, you must not insert text match org-property-end-re or
>    org-outline-regexp-bol
>
> Obviously, point 3 needs not be checked during deletion.
>
> Instead of `after-change-functions', we may use `modification-hooks' for
> deletions, and `insert-behind-hooks' for insertions. For example, we
> might add modification-hooks property to both opening and closing line,
> and `insert-behind-hooks' on all the drawer. If any of the 3 points
> above is verified, we remove all properties.
>
> Note that if we can implement something robust with text properties, we
> might use them for headlines too, for another significant speed-up.
>
> WDYT?
>
>> I think that using fontification (something similar to
>> org-fontify-drawers) instead of after-change-functions should be
>> faster.
>
> I don't think it would be faster. With `after-change-functions',
> `modification-hooks' or `insert-behind-hook', we know exactly where the
> change happened. Fontification is fuzzier. It is not instantaneous
> either.
>
> It is an option only if we cannot do something fast and accurate with
> `after-change-functions', IMO.
>

-- 
Ihor Radchenko,
PhD,
Center for Advancing Materials Performance from the Nanoscale (CAMP-nano)
State Key Laboratory for Mechanical Behavior of Materials, Xi'an Jiaotong University, Xi'an, China
Email: yantar92@gmail.com, ihor_radchenko@alumni.sutd.edu.sg

  parent reply	other threads:[~2020-05-17 15:05 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-24  6:55 Ihor Radchenko
2020-04-24  8:02 ` Nicolas Goaziou
2020-04-25  0:29   ` stardiviner
2020-04-26 16:04   ` Ihor Radchenko
2020-05-04 16:56     ` Karl Voit
2020-05-07  7:18       ` Karl Voit
2020-05-09 15:43       ` Ihor Radchenko
2020-05-07 11:04     ` Christian Heinrich
2020-05-09 15:46       ` Ihor Radchenko
2020-05-08 16:38     ` Nicolas Goaziou
2020-05-09 13:58       ` Nicolas Goaziou
2020-05-09 16:22         ` Ihor Radchenko
2020-05-09 17:21           ` Nicolas Goaziou
2020-05-10  5:25             ` Ihor Radchenko
2020-05-10  9:47               ` Nicolas Goaziou
2020-05-10 13:29                 ` Ihor Radchenko
2020-05-10 14:46                   ` Nicolas Goaziou
2020-05-10 16:21                     ` Ihor Radchenko
2020-05-10 16:38                       ` Nicolas Goaziou
2020-05-10 17:08                         ` Ihor Radchenko
2020-05-10 19:38                           ` Nicolas Goaziou
2020-05-09 15:40       ` Ihor Radchenko
2020-05-09 16:30         ` Ihor Radchenko
2020-05-09 17:32           ` Nicolas Goaziou
2020-05-09 18:06             ` Ihor Radchenko
2020-05-10 14:59               ` Nicolas Goaziou
2020-05-10 15:15                 ` Kyle Meyer
2020-05-10 16:30                 ` Ihor Radchenko
2020-05-10 19:32                   ` Nicolas Goaziou
2020-05-12 10:03                     ` Nicolas Goaziou
2020-05-17 15:00                     ` Ihor Radchenko [this message]
2020-05-17 15:40                       ` Ihor Radchenko
2020-05-18 14:35                         ` Nicolas Goaziou
2020-05-18 16:52                           ` Ihor Radchenko
2020-05-19 13:07                             ` Nicolas Goaziou
2020-05-23 13:52                               ` Ihor Radchenko
2020-05-23 13:53                                 ` Ihor Radchenko
2020-05-23 15:26                                   ` Ihor Radchenko
2020-05-26  8:33                                 ` Nicolas Goaziou
2020-06-02  9:21                                   ` Ihor Radchenko
2020-06-02  9:23                                     ` Ihor Radchenko
2020-06-02 12:10                                       ` Bastien
2020-06-02 13:12                                         ` Ihor Radchenko
2020-06-02 13:23                                           ` Bastien
2020-06-02 13:30                                             ` Ihor Radchenko
2020-06-02  9:25                                     ` Ihor Radchenko
2020-06-05  7:26                                     ` Nicolas Goaziou
2020-06-05  8:18                                       ` Ihor Radchenko
2020-06-05 13:50                                         ` Nicolas Goaziou
2020-06-08  5:05                                           ` Ihor Radchenko
2020-06-08  5:06                                             ` Ihor Radchenko
2020-06-08  5:08                                             ` Ihor Radchenko
2020-06-10 17:14                                             ` Nicolas Goaziou
2020-06-21  9:52                                               ` Ihor Radchenko
2020-06-21 15:01                                                 ` Nicolas Goaziou
2020-08-11  6:45                                               ` Ihor Radchenko
2020-08-11 23:07                                                 ` Kyle Meyer
2020-08-12  6:29                                                   ` Ihor Radchenko
2020-09-20  5:53                                                     ` Ihor Radchenko
2020-09-20 11:45                                                       ` Kévin Le Gouguec
2020-09-22  9:05                                                         ` Ihor Radchenko
2020-09-22 10:00                                                           ` Ihor Radchenko
2020-09-23  6:16                                                             ` Kévin Le Gouguec
2020-09-23  6:48                                                               ` Ihor Radchenko
2020-09-23  7:09                                                                 ` Bastien
2020-09-23  7:30                                                                   ` Ihor Radchenko
2020-09-24 18:07                                                                 ` Kévin Le Gouguec
2020-09-25  2:16                                                                   ` Ihor Radchenko
2020-12-15 17:38                                                                     ` [9.4] Fixing logbook visibility during isearch Kévin Le Gouguec
2020-12-16  3:15                                                                       ` Ihor Radchenko
2020-12-16 18:05                                                                         ` Kévin Le Gouguec
2020-12-17  3:18                                                                           ` Ihor Radchenko
2020-12-17 14:50                                                                             ` Kévin Le Gouguec
2020-12-18  2:23                                                                               ` Ihor Radchenko
2020-12-24 23:37                                                                                 ` Kévin Le Gouguec
2020-12-25  2:51                                                                                   ` Ihor Radchenko
2020-12-25 10:59                                                                                     ` Kévin Le Gouguec
2020-12-25 12:32                                                                                       ` Ihor Radchenko
2020-12-25 21:35                                                                                     ` Kévin Le Gouguec
2020-12-26  4:14                                                                                       ` Ihor Radchenko
2020-12-26 11:44                                                                                         ` Kévin Le Gouguec
2020-12-26 12:22                                                                                           ` Ihor Radchenko
2020-12-04  5:58                                                       ` [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers Ihor Radchenko
2021-03-21  9:09                                                         ` Ihor Radchenko
2021-05-03 17:28                                                           ` Bastien
2021-09-21 13:32                                                             ` Timothy
2021-10-26 17:25                                                               ` Matt Price
2021-10-27  6:27                                                                 ` Ihor Radchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87tv0efvyd.fsf@localhost \
    --to=yantar92@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=mail@nicolasgoaziou.fr \
    --subject='Re: [patch suggestion] Mitigating the poor Emacs performance on huge org files: Do not use overlays for PROPERTY and LOGBOOK drawers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).