> See also `gensym'. Do we really need to use it for something else than > `invisible'? If not, the tool doesn't need to be generic.
For now, I also use it for buffer-local 'invisible stack. The stack is needed to preserve folding state of drawers/blocks inside folded outline. Though I am thinking about replacing the stack with separate text properties, like 'invisible-outline-buffer-local + 'invisible-drawer-buffer-local + 'invisible-block-buffer-local. Maintaining stack takes a noticeable percentage of CPU time in profiler. org--get-buffer-local-text-property-symbol must take care about situation with indirect buffers. When an indirect buffer is created from some org buffer, the old value of char-property-alias-alist is carried over. We need to detect this case and create new buffer-local symbol, which is unique to the newly created buffer (but not create it if the buffer-local property is already there). Then, the new symbol must replace the old alias in char-property-alias-alist + old folding state must be preserved (via copying the old invisibility specs into the new buffer-local text property). I do not see how gensym can benefit this logic. > OK, but this may not be sufficient if we want to do slightly better than > overlays in that area. This is not mandatory, though. Could you elaborate on what can be "slightly better"? > As discussed before, I don't think you need to use `modification-hooks' > or `insert-behind-hooks' if you already use `after-change-functions'. > > `after-change-functions' are also triggered upon text properties > changes. So, what is the use case for the other hooks? The problem is that `after-change-functions' cannot be a text property. Only `modification-hooks' and `insert-in-front/behind-hooks' can be a valid text property. If we use `after-change-functions', they will always be triggered, regardless if the change was made inside or outside folded region. >> :asd: >> :drawer: >> lksjdfksdfjl >> sdfsdfsdf >> :end: >> >> If :asd: was inserted in front of folded :drawer:, changes in :drawer: >> line of the new folded :asd: drawer would reveal the text between >> :drawer: and :end:. >> >> Let me know what you think on this. > I have first to understand the use case for `modification-hook'. But > I think unfolding is the right thing to do in this situation, isn't it? That situation arises because the modification-hooks from ":drawer:" (they are set via text properties) only have information about the :drawer:...:end: drawer before the modifications (they were set when :drawer: was folded last time). So, they will only unfold a part of the new :asd: drawer. I do not see a simple way to unfold everything without re-parsing the drawer around the changed text. Actually, I am quite unhappy with the performance of modification-hooks set via text properties (I am using this patch on my Emacs during this week). It appears that setting the text properties costs a significant CPU time in practice, even though running the hooks is pretty fast. I will think about a way to handle modifications using global after-change-functions. > `org--get-element-region-at-point' is certainly faster, but it is also > wrong, unfortunately. > > Org syntax is not context-free grammar. If you try to parse it locally, > starting from anywhere, it will fail at some point. For example, your > function would choke in the following case: > > [fn:1] Def1 > #+begin_something > > [fn:2] Def2 > #+end_something I see. > AFAIK, the only proper way to parse it is to start from a known position > in the buffer. If you have no information about the buffer, the headline > above is the position you want. With cache could help to start below. > Anyway, in this particular case, you should not use > `org--get-element-region-at-point'. OK Best, Ihor Nicolas Goaziou <m...@nicolasgoaziou.fr> writes: > Hello, > > Ihor Radchenko <yanta...@gmail.com> writes: > >> [The patch itself will be provided in the following email] > > Thank you. > >> I have found char-property-alias-alist variable that controls how Emacs >> calculates text property value if the property is not set. This variable >> can be buffer-local, which allows independent 'invisible states in >> different buffers. > > Great. I didn't know about this variable! > >> All the implementation stays in >> org--get-buffer-local-text-property-symbol, which takes care about >> generating unique property name and mapping it to 'invisible (or any >> other) text property. > > See also `gensym'. Do we really need to use it for something else than > `invisible'? If not, the tool doesn't need to be generic. > >> I simplified the code as suggested, without using pairs of before- and >> after-change-functions. > > Great! > >> Handling text inserted into folded/invisible region is handled by a >> simple after-change function. After testing, it turned out that simple >> re-hiding text based on 'invisible property of the text before/after the >> inserted region works pretty well. > > OK, but this may not be sufficient if we want to do slightly better than > overlays in that area. This is not mandatory, though. > >> Modifications to BEGIN/END line of the drawers and blocks is handled via >> 'modification-hooks + 'insert-behind-hooks text properties (there is no >> after-change-functions analogue for text properties in Emacs). The >> property is applied during folding and the modification-hook function is >> made aware about the drawer/block boundaries (via apply-partially >> passing element containing :begin :end markers for the current >> drawer/block). Passing the element boundary is important because the >> 'modification-hook will not directly know where it belongs to. Only the >> modified region (which can be larger than the drawer) is passed to the >> function. In the worst case, the region can be the whole buffer (if one >> runs revert-buffer). > > As discussed before, I don't think you need to use `modification-hooks' > or `insert-behind-hooks' if you already use `after-change-functions'. > > `after-change-functions' are also triggered upon text properties > changes. So, what is the use case for the other hooks? > >> It turned out that adding 'modification-hook text property takes a >> significant cpu time (partially, because we need to take care about >> possible existing 'modification-hook value, see >> org--add-to-list-text-property). For now, I decided to not clear the >> modification hooks during unfolding because of poor performance. >> However, this approach would lead to partial unfolding in the following >> case: >> >> :asd: >> :drawer: >> lksjdfksdfjl >> sdfsdfsdf >> :end: >> >> If :asd: was inserted in front of folded :drawer:, changes in :drawer: >> line of the new folded :asd: drawer would reveal the text between >> :drawer: and :end:. >> >> Let me know what you think on this. > > I have first to understand the use case for `modification-hook'. But > I think unfolding is the right thing to do in this situation, isn't it? > >> My simplified implementation of element boundary parser >> (org--get-element-region-at-point) appears to be much faster and also >> uses much less memory in comparison with org-element-at-point. >> Moreover, not all the places where org-element-at-point is called >> actually need the full parsed element. For example, org-hide-drawer-all, >> org-hide-drawer-toggle, org-hide-block-toggle, and >> org--hide-wrapper-toggle only need element type and some information >> about the element boundaries - the information we can get from >> org--get-element-region-at-point. > > [...] > >> What do you think about the idea of making use of >> org--get-element-region-at-point in org code base? > > `org--get-element-region-at-point' is certainly faster, but it is also > wrong, unfortunately. > > Org syntax is not context-free grammar. If you try to parse it locally, > starting from anywhere, it will fail at some point. For example, your > function would choke in the following case: > > [fn:1] Def1 > #+begin_something > > [fn:2] Def2 > #+end_something > > AFAIK, the only proper way to parse it is to start from a known position > in the buffer. If you have no information about the buffer, the headline > above is the position you want. With cache could help to start below. > Anyway, in this particular case, you should not use > `org--get-element-region-at-point'. > > Hopefully, we don't need to parse anything. In an earlier message, > I suggested a few checks to make on the modified text in order to decide > if something should be unfolded, or not. I suggest to start from there, > and fix any shortcomings we might encounter. We're replacing overlays: > low-level is good in this area. > > WDYT? > > > Regards, > > -- > Nicolas Goaziou -- 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: yanta...@gmail.com, ihor_radche...@alumni.sutd.edu.sg
