Re: [9.4] Fixing logbook visibility during isearch
Kévin Le Gouguec writes: > Ihor Radchenko writes: > >> Kévin Le Gouguec writes: >> >>> 1.2. stumps me: is there an isearch API I can use while in the callback >>> to know where the matches are located? >> >> I do not think that there is direct API for this, but the match should >> be accessible through match-beginning/match-end, as I can see from the >> isearch.el code. > > Right, I've seen this too; I wonder if it's a hard guarantee or an > implementation detail. I might page help-gnu-emacs about this. Another way could by using isearch-filter-predicate. It is given the search region directly.
Re: [9.4] Fixing logbook visibility during isearch
Ihor Radchenko writes: > Kévin Le Gouguec writes: > >> 1.2. stumps me: is there an isearch API I can use while in the callback >> to know where the matches are located? > > I do not think that there is direct API for this, but the match should > be accessible through match-beginning/match-end, as I can see from the > isearch.el code. Right, I've seen this too; I wonder if it's a hard guarantee or an implementation detail. I might page help-gnu-emacs about this.
Re: [9.4] Fixing logbook visibility during isearch
Kévin Le Gouguec writes: > 1.2. stumps me: is there an isearch API I can use while in the callback > to know where the matches are located? I do not think that there is direct API for this, but the match should be accessible through match-beginning/match-end, as I can see from the isearch.el code. > For 2.1, I guess we will need to cache the temporary invisible overlays > we add during step 1. in a global list; that way when it's time to > destroy them, we can simply iterate on the list? That's what I do in org-fold--isearch-show-temporary. Best, Ihor
Re: [9.4] Fixing logbook visibility during isearch
Ihor Radchenko writes: > However, org-cycle-hide-drawers might be called in > isearch-open-invisible-temporary. This callback receives two arguments: - the overlay which contains a match, - whether we are un-hiding the overlay's span or hiding it back. To get the same behaviour as Org≤9.3, IIUC we want to do the following: 1. When isearch asks us to un-hide, 1. go over all drawers within the overlay, 2. hide those that do not contain a match, by adding an invisible overlay. 2. When isearch asks us to hide back, 1. remove the invisible overlays we have put on these drawers. 1.1. is straightforward: overlay-start and overlay-end tell us where to look for drawers. 1.2. stumps me: is there an isearch API I can use while in the callback to know where the matches are located? For 2.1, I guess we will need to cache the temporary invisible overlays we add during step 1. in a global list; that way when it's time to destroy them, we can simply iterate on the list? (Sorry for being so slow I never seem to be able to spend more than 10 minutes on this issue before having to switch to something else…)
Re: [9.4] Fixing logbook visibility during isearch
Kévin Le Gouguec writes: > Ihor Radchenko writes: > >> My current plan is supporting the overlay-based approach even after >> merging the branch (by default). So, overlays should be around for a >> while and the issue with drawer visibility will be around as well, >> unless you fix it. I will probably work on this in distant future, but >> that's not the priority now. > > Mmm; is the current state of your branch representative of your plan? Not yet. That's rather a big change and I am currently generalising the core org-fold API to support both text properties and overlays. You can see WIP in org-fold-core.el from org-fold-universal-core branch. That branch is not usable yet. Best, Ihor
Re: [9.4] Fixing logbook visibility during isearch
Ihor Radchenko writes: > My current plan is supporting the overlay-based approach even after > merging the branch (by default). So, overlays should be around for a > while and the issue with drawer visibility will be around as well, > unless you fix it. I will probably work on this in distant future, but > that's not the priority now. Mmm; is the current state of your branch representative of your plan? If I compile it and run emacs -Q -L $yourbranch/lisp --eval '(setq org-startup-folded t)' $someorgfile Then isearching does not reveal logbook drawers unless matches are found inside, which as far as I am concerned fixes my issue with 9.4. >> I wonder if the path of least resistance couldn't be found in >> org-cycle-hide-drawers: right now this function just skips over drawers >> which are covered with an invisible overlay, but maybe it should not >> skip a drawer if the overlay starts before it (i.e. the overlay is not >> specific to this drawer but covers a whole containing section). > > That would defeat the purpose why the number of overlays was reduced in > Org 9.4. However, org-cycle-hide-drawers might be called in > isearch-open-invisible-temporary. Thanks for the tip.
Re: [9.4] Fixing logbook visibility during isearch
Kévin Le Gouguec writes: > I've looked at your branch for inspiration, and my takeaway is that the > isearch-open-invisible-temporary route might be too involved for a > bugfix, especially if it's going to be reverted wholesale when your > branch gets merged. Then again, maybe I'm not smart enough to devise a > solution. My current plan is supporting the overlay-based approach even after merging the branch (by default). So, overlays should be around for a while and the issue with drawer visibility will be around as well, unless you fix it. I will probably work on this in distant future, but that's not the priority now. > I wonder if the path of least resistance couldn't be found in > org-cycle-hide-drawers: right now this function just skips over drawers > which are covered with an invisible overlay, but maybe it should not > skip a drawer if the overlay starts before it (i.e. the overlay is not > specific to this drawer but covers a whole containing section). That would defeat the purpose why the number of overlays was reduced in Org 9.4. However, org-cycle-hide-drawers might be called in isearch-open-invisible-temporary. Best, Ihor
Re: [9.4] Fixing logbook visibility during isearch
Ihor Radchenko writes: > Kévin Le Gouguec writes: > >> Since the changes in Org 9.4 aimed at improving performance, is there a >> test case somewhere in the "Mitigating the poor Emacs performance on >> huge org files" thread that could help ensure that a tentative fix will >> not degrade performance? > > The first message in the thread ;) I believe it was also used to > benchmark the change in 9.4. Thanks for the pointer! I've looked at your branch for inspiration, and my takeaway is that the isearch-open-invisible-temporary route might be too involved for a bugfix, especially if it's going to be reverted wholesale when your branch gets merged. Then again, maybe I'm not smart enough to devise a solution. I wonder if the path of least resistance couldn't be found in org-cycle-hide-drawers: right now this function just skips over drawers which are covered with an invisible overlay, but maybe it should not skip a drawer if the overlay starts before it (i.e. the overlay is not specific to this drawer but covers a whole containing section).
Re: [9.4] Fixing logbook visibility during isearch
Kévin Le Gouguec writes: > Since the changes in Org 9.4 aimed at improving performance, is there a > test case somewhere in the "Mitigating the poor Emacs performance on > huge org files" thread that could help ensure that a tentative fix will > not degrade performance? The first message in the thread ;) I believe it was also used to benchmark the change in 9.4. >> [3] See the attached org file in my Emacs bug report: >> https://lists.gnu.org/archive/html/bug-gnu-emacs/2019-04/txte6kQp35VOm.txt Or you can ask me to test. That example file is my stripped someday list, which grew to much larger size since the time I created that example. Best, Ihor
Re: [9.4] Fixing logbook visibility during isearch
Ihor Radchenko writes: > You will probably need to implement this from scratch (or use the > feature/org-fold branch from github.com/yantar92/org). Gotcha. TBH I don't know if I'll have the time to cook up a patch before 27.2 is released; all the same, I appreciate you taking the time to explain all this. Since the changes in Org 9.4 aimed at improving performance, is there a test case somewhere in the "Mitigating the poor Emacs performance on huge org files" thread that could help ensure that a tentative fix will not degrade performance?
Re: [9.4] Fixing logbook visibility during isearch
Kévin Le Gouguec writes:
> I can't find any reference to this property in Org <9.4 (e.g. 9.3 as
> shipped in 27.1, where the bug does not happen) so do I understand
> correctly that the root cause ("since [drawers] are in the same overlay
> with the rest of the folded heading") dates from Org 9.4?
Yes, the root cause is that overlays used to hide drawers now
automatically merge with outline overlays. This was introduced in Org
9.4 to improve performance (too many overlays are handled badly by
Emacs).
> (Just trying to understand if I should keep looking at Org 9.3 for
> inspiration, or if your proposed solution based on
> isearch-open-invisible-temporary should be implemented from scratch)
You will probably need to implement this from scratch (or use the
feature/org-fold branch from github.com/yantar92/org).
In Org 9.3 the folded headline looked like the following:
* Headline
:PROPERTIES:
:PROPERTY1: value1
:PROPERTY2: value2
:END:
headline text
another line
When using isearch with "text" search string, the overlay containing
"text" is temporarily revealed by isearch (via setting 'invisible
property of the overlay to nil):
* Headline
:PROPERTES:
:PROPERTY1: value1
:PROPERTY2: value2
:END:
headline text
another line
As you can see, the drawer overlay remains unchanged and hidden.
In Org 9.4, drawer overlay does not exist when we fold the headline text
and isearch reveals everything.
To work around this issue, you need to hook into the way isearch reveals
hidden match by setting 'isearch-open-invisible-temporary property of
the overlays to custom function (you can set the property inside
org-flag-region). The function should re-hide the drawers when matching
text is not inside the drawer.
Best,
Ihor
Re: [9.4] Fixing logbook visibility during isearch
Ihor Radchenko writes:
> Kévin Le Gouguec writes:
>
>> The debugger only fires *after* we exit isearch, and by that time it's
>> too late: my issue comes from all those logbooks cluttering the screen
>> while I'm mashing C-s to iterate through matches.
>>
>> I can try to dig deeper into this, but before doing so: would you have
>> any insight as to what's going on here?
>
> org-mode is relying on default isearch behaviour during interactive C-s
> session. By default, isearch simply makes all the overlays at match
> visible and re-hide them once we move to the next match. In case of
> org-mode, this reveals drawers as well, since they are in the same
> overlay with the rest of the folded heading.
>
> The way to change default isearch behaviour *during* isearch session is
> setting undocumented 'isearch-open-invisible-temporary property of the
> overlay (see isearch-open-overlay-temporary).
Thanks for taking the time to explain this.
I can't find any reference to this property in Org <9.4 (e.g. 9.3 as
shipped in 27.1, where the bug does not happen) so do I understand
correctly that the root cause ("since [drawers] are in the same overlay
with the rest of the folded heading") dates from Org 9.4?
(Just trying to understand if I should keep looking at Org 9.3 for
inspiration, or if your proposed solution based on
isearch-open-invisible-temporary should be implemented from scratch)
Re: [9.4] Fixing logbook visibility during isearch
Kévin Le Gouguec writes: > The debugger only fires *after* we exit isearch, and by that time it's > too late: my issue comes from all those logbooks cluttering the screen > while I'm mashing C-s to iterate through matches. > > I can try to dig deeper into this, but before doing so: would you have > any insight as to what's going on here? org-mode is relying on default isearch behaviour during interactive C-s session. By default, isearch simply makes all the overlays at match visible and re-hide them once we move to the next match. In case of org-mode, this reveals drawers as well, since they are in the same overlay with the rest of the folded heading. The way to change default isearch behaviour *during* isearch session is setting undocumented 'isearch-open-invisible-temporary property of the overlay (see isearch-open-overlay-temporary). The function must accept two arguments: overlay and flag. If flag is non-nil, the function should re-hide the overlay text and it should reveal the overlay when flag is nil. Best, Ihor
[9.4] Fixing logbook visibility during isearch
Ihor Radchenko writes: > However, I can try to suggest a way to fix the issue on master. The way > isearch handles folded text in org is set from org-flag-region > (org-macs.el): > > (overlay-put o > 'isearch-open-invisible > (lambda ( _) (org-show-context 'isearch))) > > It means that isearch calls org-show-context (org.el) to reveal hidden > text. Then, it calls org-show-set-visibility with argument defined in > org-show-context-detail (now, it is 'lineage). With current defaults, > the searched text is revealed using org-flag-heading, which reveals both > heading body and drawers. > > The easiest way to write the fix would be changing org-flag-heading > directly, but there might be unforeseen consequences on other folding > commands. > > Another way would be changing the way org-show-set-visibility handles > 'lineage argument. Again, it may affect other things. > > Finally, one can add an extra possible argument to > org-show-set-visibility and alter default value of > org-show-context-detail accordingly. > > The last way will have least risk to break something else. > > I guess, patches welcome ;) Since Org 9.4 has landed in the emacs-27 branch, I have renewed interest in finding a fix for this before 27.2 is released (… and more selfishly, before emacs-27 is merged into master ). I'm a bit confused, because AFAICT org-show-context is called *after* exiting isearch, so IIUC by the time org-show-set-visibility is called it's too late to undo the damage. Recipe using my repro file[1]: - C-x C-f logbooks.org - M-x toggle-debug-on-entry org-show-context - C-s bug The debugger only fires *after* we exit isearch, and by that time it's too late: my issue comes from all those logbooks cluttering the screen while I'm mashing C-s to iterate through matches. I can try to dig deeper into this, but before doing so: would you have any insight as to what's going on here? [1] wget https://orgmode.org/list/87eepuz0bj@gmail.com/2-logbooks.org -O tmp/logbooks.org
