org-appear icon indicating copy to clipboard operation
org-appear copied to clipboard

org-appear doesn't work with experimental org-mode branch feature/org-fold

Open AloisJanicek opened this issue 4 years ago • 25 comments

I am a happy user of yantar92/org's feature/org-fold branch because of its noticeable performance boost with big org files, however I just noticed that org-appear doesn't work with it.

"doesn't work" here means that after I enable org-appear-mode and place cursor over word surrounded by some org-mode emphasis, they aren't revealed, they stay hidden.

When I switch to normal org-mode, issue is gone and org-appear works as intended, so I am confident to say that the issue must be either in the feature/org-fold branch itself or in theory issue could have been introduced to org-mode somewhere after 6b83c6e4ea, because that's the current version of vanilla org-mode used in doom and org-appear works fine with it.

I tested this on stable emacs 27 and vanilla doom emacs profile.

AloisJanicek avatar Jan 31 '21 20:01 AloisJanicek

As far as I can see, from a cursory look at the the feature/org-fold branch, it uses a different mechanism for hiding emphasis markers in org-do-emphasis-faces. I'll have to do some reading/testing to see if this change in Org mode source completely breaks org-appear or if there is a fix that won't require a separate code path to support feature/org-fold.

awth13 avatar Feb 01 '21 06:02 awth13

Maybe just summon @yantar92 here.

He is really fast and helpful to resolve issues regarding his branch.

dakra avatar Feb 01 '21 07:02 dakra

=feature/org-fold= does not hide emphasis markers using invisible text property. The visibility is set using alternative "buffer-local" text properties. To switch visibility of emphasis marker in a region, I recommend to use =font-lock-fontify-region= with locally bound =org-hide-emphasis-markers= (see =org-fold-show-set-visibility= for an example how it can be done). This approach should work on master as well.

yantar92 avatar Feb 01 '21 14:02 yantar92

Thank you, @dakra and @yantar92! Even though it doesn't solve the issue entirely -- org-appear also wants to support link and sub/superscript toggling -- it's definitely useful to be aware of this approach. It does work on both master and feature/org-fold.

awth13 avatar Feb 02 '21 05:02 awth13

org-appear also wants to support link and sub/superscript toggling

I do not see why font-lock-fontify-region would not work here. Just need to bind org-use-sub-superscripts in addition to org-hide-emphasis-markers.

yantar92 avatar Feb 02 '21 05:02 yantar92

Yes, I managed to make it work with sub/superscripts by locally binding org-pretty-entites -- although it toggles the entire line rather than a single fragment, working on it -- but no luck with links so far.

awth13 avatar Feb 02 '21 07:02 awth13

but no luck with links so far.

Binding org-link-descriptive does not help?

yantar92 avatar Feb 02 '21 07:02 yantar92

Binding org-link-descriptive does not help?

Nope, at least on master, it doesn't. Incidentally, do you know of a better way to prevent font-lock from extending the region to whole line than binding font-lock-extend-region-functions?

awth13 avatar Feb 02 '21 07:02 awth13

do you know of a better way to prevent font-lock from extending the region to whole line than binding font-lock-extend-region-functions

Judging from the source code of font-lock-fontify-region, setting font-lock-extend-region-functions should be the right way.

Nope, at least on master, it doesn't.

After thinking, it makes sense. The problem is how org-link-descriptive is implemented. It is used via buffer invisibility specs, so that removing org-link from the specs automatically reveal all the hidden parts of the links without a need to update the text properties. The problem is that buffer invisibility specs are applied to the whole buffer. So, you may have to rely on implementation details (different in org-fold and on master).

yantar92 avatar Feb 02 '21 08:02 yantar92

Thanks a lot for the discussion, @yantar92!

The version of org-appear that supports both branches is implemented here. There are two issues to resolve before it can be merged:

  1. I haven't come up with a good way to support link toggling on org-fold so far. The problem is, indeed, in how org-link-descriptive is implemented.
  2. Using font-lock-fontify-region instead of modifying text properties means that org-appear toggles nested children when it enters a parent. I currently don't see a solution for this other than using RegEx to find and "untoggle" children but this is hacky and I don't want to use RegEx -- running hooks after each command is a performance tax as it is. This "issue" isn't necessarily a bad thing, just not how I imagined org-appear would work. Any input is welcome!

awth13 avatar Feb 03 '21 07:02 awth13

I haven't come up with a good way to support link toggling on org-fold so far. The problem is, indeed, in how org-link-descriptive is implemented.

To reveal a link in org-fold, you can simply run (org-fold-show-set-visibility 'local) with point inside the link.

Using font-lock-fontify-region instead of modifying text properties means that org-appear toggles nested children when it enters a parent.

Do you refer to nested emphasis? org-fold will support it eventually as a part of handling invisible edits. It is in my todo-list. It will be implemented using org-fold-show-set-visibility.

yantar92 avatar Feb 04 '21 04:02 yantar92

you can simply run (org-fold-show-set-visibility 'local) with point inside the link

I tried that but it seemed to mess with link faces -- only the description part is properly highlighted, the rest of the link is displayed as normal text. I now realise that this may be an issue of feature/org-fold in general since org-toggle-link-display produces the same behaviour on the branch.

Do you refer to nested emphasis? org-fold will support it eventually

Sorry, I probably meant to say "nested emphasis children". Could you please clarify about the thing on your todo list? What's it going to do exactly?

awth13 avatar Feb 04 '21 07:02 awth13

I tried that but it seemed to mess with link faces -- only the description part is properly highlighted, the rest of the link is displayed as normal text.

Fixed.

Sorry, I probably meant to say "nested emphasis children". Could you please clarify about the thing on your todo list? What's it going to do exactly?

I meant something like *beginning of bold text, then /italic text*, italics still present after bold text ends/. The plan is revealing emphasis markers (both) when user attempts to remove one of the invisible markers. Currently, all the markers in a continious emphasised substring are revealed (even when not edited) - if the user tries to remove the first "*", the "/"-s are also revealed. I plan to fix it.

yantar92 avatar Feb 04 '21 10:02 yantar92

Support for links is now added in org-fold-support branch.

I'm not sure if we're talking about the same thing, @yantar92. What I meant is this.

In the version of org-appear currently on master, because it modifies text properties around emphasis markers only, toggling a parent doesn't affect children:

  • actual text :: *parent and /child/ nested*
  • cursor somewhere else :: parent and child nested
  • cursor entered parent :: *parent and child nested*

In the version that uses font-lock-fontify-region, the entire region inside parent is fontified:

  • cursor somewhere else :: parent and child nested
  • cursor entered parent :: *parent and /child/ nested*

awth13 avatar Feb 04 '21 13:02 awth13

I'm not sure if we're talking about the same thing, @yantar92. What I meant is this.

I also meant that kind of situation. Though the example I provided is probably even more tricky. Would will happen if you have *left part /inters<!>ection* right part/ with <!> indicating cursor position?

yantar92 avatar Feb 04 '21 13:02 yantar92

Answering my own question:

  • actual text :: /this *bold is/ and more bold*
  • cursor entering first part of the text :: /t<!>his *bold is/ and more bold*
  • cursor entering last part of the text :: this bold is <!>and more bold

Emphasis markers are not revealed in the last case, because org-element-context does not recognise emphasis there...

yantar92 avatar Feb 04 '21 14:02 yantar92

Who in their right mind would nest emphasis like that?

Jokes aside, yes, this is trickier. There's actually a bug related to that right now in org-appear -- I didn't even think to check for that before you mentioned it.

awth13 avatar Feb 04 '21 15:02 awth13

Not on main yet. I need maintainers to agree about the merge. It is a major change. I plan to prepare a proper patchset and bump the thread again a few weeks later, when the dust settles on the recent org-persist/org-element merge.

Seems like feature/org-fold will be merged to the main soon.

https://list.orgmode.org/877ddzyocg.fsf@localhost/

willbchang avatar Oct 31 '21 02:10 willbchang

Seems like feature/org-fold will be merged to the main soon.

For the record, org-appear works fine with org-fold (at least for emphasis). The problem with intersecting emphasis is not real because they are not really supported by Org. Fontification works by accident...

yantar92 avatar Oct 31 '21 02:10 yantar92

Thanks, @willbchang and @yantar92, for the information!

org-appear works fine with org-fold (at least for emphasis)

The only element currently not supported with org-fold is links. I will work on resolving that before org-fold is merged.

awth13 avatar Oct 31 '21 12:10 awth13

FYI, org-fold has been merged. yantar92/org#42

yantar92 avatar May 02 '22 04:05 yantar92

@yantar92 Is there an easy way to temporarily unfold lines for a swiper-like search? I need a function to unfold which also gives me a restoration function as return value, such that I can fold again afterwards. And then I need a function to unfold finally (maybe by just not calling the restoration function again.) Does org-fold come with such a functionality? I saw there is org-fold-show-context.

minad avatar May 02 '22 21:05 minad

Is there an easy way to temporarily unfold lines for a swiper-like search?

(org-fold-show-set-visibility 'local)

I need a function to unfold which also gives me a restoration function as return value, such that I can fold again afterwards. And then I need a function to unfold finally (maybe by just not calling the restoration function again.) Does org-fold come with such a functionality? I saw there is org-fold-show-context.

This is more tricky. There is currently no API function to do this (though link/emphasis visibility is restored upon re-fontification). You can use org-fold-core--isearch-show-temporary used to implement temporary visibility for isearch, but it may be a subject of change in future.

If you want to add this functionality officially (it was never present in the past, AFAIK), feel free to open a discussion in Org ML. Or I was planning to talk about some Org internals during next Org Profiling Meetup (previous meetup: @.***). We may discuss org-fold-core internals if you are interested.

yantar92 avatar May 03 '22 01:05 yantar92

@yantar92 Thanks! This is helpful. A simple official API for unfolding and subsequent restoration would be nice to have. Such an API didn't exist previously since Org just relied on the standard Isearch unfolding mechanism for overlay used by outline and hideshow.

minad avatar May 03 '22 04:05 minad

The mainline version of org-appear now supports all elements with org-fold!

I am not closing the issue for now since there are conflicts with packages using font-lock and jit-lock functions, as described in #34.

awth13 avatar Jun 18 '22 00:06 awth13