From: Ihor Radchenko <firstname.lastname@example.org> To: Nicolas Goaziou <email@example.com> Cc: firstname.lastname@example.org Subject: Re: Question about Org syntax Date: Sun, 23 May 2021 11:50:07 +0800 [thread overview] Message-ID: <875yzap0r4.fsf@localhost> (raw) In-Reply-To: <email@example.com> [-- Attachment #1: Type: text/plain, Size: 4998 bytes --] Nicolas Goaziou <firstname.lastname@example.org> writes: Thanks for your detailed explanations! >> 2. Some of the element parsers honour LIMIT argument partially. Part of >> the parsing is typically done using looking-at (ignoring the LIMIT) >> and part is honouring it. This can backfire when LIMIT is before >> first characteristic line of the element. For example take headline >> parser: >> ... > > LIMIT is not a random position in the buffer. It is supposed to be the > end of the parent element, or (point-max). > > It is a bug (in the parser or in the cache) if it ends up being anywhere > else. Makes sense, though it is not mentioned in the docstrings and even in the docstring of the `org-element--current-element'. Moreover, section comment about parsers states that most parsers accept no arguments: > ;; A parser returns the element or object as the list described above. > ;; Most of them accepts no argument. And the comment about adding things to `org-element--current-element' does not help either. > ;; Beside implementing a parser and an interpreter, adding a new > ;; greater element requires tweaking `org-element--current-element'. > ;; Moreover, the newly defined type must be added to both > ;; `org-element-all-elements' and `org-element-greater-elements'. Probably, the docstring of `org-element--current-element' could mention about the expected values of LIMIT argument? ------ Also, I have some more questions as I am trying to understand the org-element-cache code. I tried to add some additional comments to the existing code to clarify parts I had difficulties understanding. See the attached patch. Let me know if any of the comments are incorrect so that I can update my understanding. For now I am stuck understanding some places in the code. They are either bugs or I misunderstood some things: 1. In org-element--cache-process-request: > (while t > ... > (let ((beg (aref request 0)) > (end (aref request 2)) > (node (org-element--cache-root)) > data data-key last-container) > > ... > (and last-container > ... > (progn (when (and (not last-container) > (> (org-element-property :end data) > end)) > (setq last-container data)) I do not understand the use of last-container here. The code is ran in a while loop containing let-bound last-container variable (the let binding sets it to nil). (setq last-container data) will not affect further iterations of the while loop as last-container will always be nil inside the let-binding. 2. In org-element--cache-submit-request > (let ((first (org-element--cache-for-removal beg end offset))) > ... > (push (let ((beg (org-element-property :begin first)) > (key (org-element--cache-key first))) > ... > ((let ((first-end (org-element-property :end first))) > (and (>= first-end end) > (vector key beg first-end offset first 0)))) > ... > org-element--cache-sync-requests) According to the docstring of org-element--cache-sync-requests, (aref 4 request) during "0" phase should be "PARENT, when non-nil, is the parent of the first element to be removed. Yet, KEY is the key of the FIRST and FIRST itself is passed as PARENT. > (push (let ((beg (org-element-property :begin first)) > (key (org-element--cache-key first))) > ... > ;; Otherwise, we find the first non robust > ;; element containing END. All elements between > ;; FIRST and this one are to be removed. > ... > (t > (let* ((element (org-element--cache-find end)) > (end (org-element-property :end element)) > (up element)) > (while (and (setq up (org-element-property :parent up)) > (>= (org-element-property :begin up) beg)) > (setq end (org-element-property :end up) > element up)) > (vector key beg end offset element 0))))) Despite what the comment states, the following code simply searches for the parent of FIRST and extents the region of phase 0 request all the way to the end of that parent. (note that BEG is re-bound to the beginning of the FIRST). Now, consider the following branch of org-element--cache-submit-request. The (aref next 4) may contain the parent of FIRST from the above code. That parent is robust (or it would be returned by org-element--cache-for-removal) and its :end/:contents-end have already been shifted in the earlier call to org-element--cache-for-removal (that was where we have found the FIRST value): > ;; If last change happened within area to be removed, extend > ;; boundaries of robust parents, if any. Otherwise, find > ;; first element to remove and update request accordingly. > (if (> beg delete-from) > ... > (let ((up (aref next 4))) > (while up > (org-element--cache-shift-positions > up offset '(:contents-end :end)) > (setq up (org-element-property :parent up)))) The first iteration of the while loop will potentially extend the :end/:contents-end of the parent of FIRST second time in a row. Best, Ihor [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-comments-to-org-element-cache-code.patch --] [-- Type: text/x-diff, Size: 5163 bytes --] From 354275e12ca86b3f226a08dd57f0794e680afc49 Mon Sep 17 00:00:00 2001 Message-Id: <email@example.com> From: Ihor Radchenko <firstname.lastname@example.org> Date: Sun, 23 May 2021 11:03:40 +0800 Subject: [PATCH] Add comments to org-element-cache code --- lisp/org-element.el | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/lisp/org-element.el b/lisp/org-element.el index ba4f0ead6..b9bb87b66 100644 --- a/lisp/org-element.el +++ b/lisp/org-element.el @@ -5322,6 +5322,9 @@ (defun org-element--cache-process-request (if (if (or (not next) (org-element--cache-key-less-p data-key next)) (<= pos end) + ;; FIXME: Isn't LAST-CONTAINER always nil here? + ;; We are inside let-binding defining + ;; last-container to be nil. (and last-container (let ((up data)) (while (and up (not (eq up last-container))) @@ -5675,7 +5678,10 @@ (defun org-element--cache-for-removal (beg end offset) any position between BEG and END. As an exception, greater elements around the changes that are robust to contents modifications are preserved and updated according to the -changes." +changes. In the latter case, the returned element is the outermost +non-robust element affected by changed. Note that the returned +element may end before END position in which case some cached element +starting after the returned may still be affected by the changes." (let* ((elements (org-element--cache-find (1- beg) 'both)) (before (car elements)) (after (cdr elements))) @@ -5722,22 +5728,39 @@ (defun org-element--cache-submit-request (beg end offset) (let ((next (car org-element--cache-sync-requests)) delete-to delete-from) (if (and next + ;; First existing sync request is in phase 0. (zerop (aref next 5)) + ;; Current changes intersect with the first sync request. (> (setq delete-to (+ (aref next 2) (aref next 3))) end) (<= (setq delete-from (aref next 1)) end)) ;; Current changes can be merged with first sync request: we ;; can save a partial cache synchronization. (progn + ;; Update OFFSET of the existing request. (cl-incf (aref next 3) offset) ;; If last change happened within area to be removed, extend ;; boundaries of robust parents, if any. Otherwise, find ;; first element to remove and update request accordingly. (if (> beg delete-from) + ;; The current modification is completely inside NEXT. + ;; (aref next 4) is supposed to be the outermost parent + ;; to be removed. Everything in cache inside that + ;; element will be removed later and we do not care + ;; about extending boundaries of robust elements in + ;; there. Also, all the upper elements are guaranteed + ;; to be robust because otherwise `org-element--cache-for-removal' + ;; would return more outer element (to be stored in + ;; (aref next 4). (let ((up (aref next 4))) (while up (org-element--cache-shift-positions up offset '(:contents-end :end)) (setq up (org-element-property :parent up)))) + ;; The current and NEXT modifications are intersecting + ;; with current modification starting before NEXT and NEXT + ;; ending after current. We need to update the common + ;; non-robust parent for the new extended modification + ;; region. (let ((first (org-element--cache-for-removal beg delete-to offset))) (when first (aset next 0 (org-element--cache-key first)) @@ -5762,8 +5785,15 @@ (defun org-element--cache-submit-request (beg end offset) ;; Otherwise, we find the first non robust ;; element containing END. All elements between ;; FIRST and this one are to be removed. + ;; + ;; The current modification is completely inside + ;; FIRST. Clear and update cached elements in + ;; region containing FIRST. ((let ((first-end (org-element-property :end first))) - (and (> first-end end) + (and (>= first-end end) + ;; FIXME: According to `org-element--cache-sync-requests' + ;; docstring, (aref 4 request) must be the + ;; parent of the first element to be removed. (vector key beg first-end offset first 0)))) (t (let* ((element (org-element--cache-find end)) @@ -5773,6 +5803,8 @@ (defun org-element--cache-submit-request (beg end offset) (>= (org-element-property :begin up) beg)) (setq end (org-element-property :end up) element up)) + ;; Extend region to remove elements to parent + ;; of the FIRST. (vector key beg end offset element 0))))) org-element--cache-sync-requests) ;; No element to remove. No need to re-parent either. -- 2.26.3
prev parent reply other threads:[~2021-05-23 3:45 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-16 3:38 Ihor Radchenko 2021-05-16 7:11 ` Nicolas Goaziou 2021-05-16 8:01 ` Ihor Radchenko 2021-05-16 8:38 ` Nicolas Goaziou 2021-05-23 3:50 ` Ihor Radchenko [this message]
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=875yzap0r4.fsf@localhost \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: Question about Org syntax' \ /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).