awth13 / org-appear

Toggle visibility of hidden Org mode element parts upon entering and leaving an element
MIT License
369 stars 19 forks source link

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

Open AloisJanicek opened 3 years ago

AloisJanicek commented 3 years ago

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.

awth13 commented 3 years ago

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.

dakra commented 3 years ago

Maybe just summon @yantar92 here.

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

yantar92 commented 3 years ago

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

awth13 commented 3 years ago

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.

yantar92 commented 3 years ago

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.

awth13 commented 3 years ago

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.

yantar92 commented 3 years ago

but no luck with links so far.

Binding org-link-descriptive does not help?

awth13 commented 3 years ago

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?

yantar92 commented 3 years ago

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

awth13 commented 3 years ago

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!
yantar92 commented 3 years ago

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.

awth13 commented 3 years ago

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?

yantar92 commented 3 years ago

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.

awth13 commented 3 years ago

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:

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

yantar92 commented 3 years ago

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 commented 3 years ago

Answering my own question:

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

awth13 commented 3 years ago

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.

willbchang commented 2 years ago

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/

yantar92 commented 2 years ago

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

awth13 commented 2 years ago

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.

yantar92 commented 2 years ago

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

minad commented 2 years ago

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

yantar92 commented 2 years ago

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.

minad commented 2 years ago

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

awth13 commented 2 years ago

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.