Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Entity Toggling #11

Merged
merged 3 commits into from
Apr 1, 2021
Merged

Entity Toggling #11

merged 3 commits into from
Apr 1, 2021

Conversation

SPFabGerman
Copy link
Contributor

Based upon the request from @japhir at #2, I implemented a very simple first approach of entity toggling. It seemed to work fine, but I did not really had the time to further test this, so there might still be some bugs.
I tried not to write a new function for this, but instead implemented this with the functionality currently available, reusing as much code as possible.

But I have found one bug: After disabling the Org-Appear Mode, the cursor behaves wierdly, when moving over previously decomposed entities. I don't really know what is causing this, since the composition seems to be reapplied correctly, so the bug is maybe not even related to Org-Mode, but instead to movement functions? Maybe a look at prettify-symbols-mode (which does more or less the same thing as Entity Toggling) will help to understand what is happening here.

@awth13
Copy link
Owner

awth13 commented Apr 1, 2021

Thank you, @SPFabGerman! Didn't expect you to jump in so quickly.

Not sure what causes this bug -- based on my testing, doesn't seem to be our abuse of font-lock-flush and font-lock-ensure to prevent autoedits -- but there is an easy fix I came up with. Just (goto-char start) when disabling an entity (you have to check that it's an entity and not another element). I can add this myself after merging but feel free to implement it.

Apart from that, I don't see any issues so far. One suggestion I have is to skip these lines entirely. pcase evaluates to nil when there is no match, and I think it's fine that entities won't have any visible start/end. Would you mind changing that before we merge?

@SPFabGerman
Copy link
Contributor Author

Hey, thank you. The suggestion with goto-char worked seemlessly.
In order to remove the unused visible-start/end properties I added some conditional checks to handle entities seperatly. (Therefor I had to include the entity-type variable in the show and hide functions too.)

But while testing, I found another very wierd bug. When using Org-Appear Mode, but without Entity Toggling, the cursor moves wierdly over the Entites. I found that point is changed when moving, so it looks like Emacs tries to move over the "real" text and not the composed Entities, which it shouldn't do. But I think this issue might be unrelated to this Pull Request, since the same thing happens in the Master Branch. I don't know what is causing this, since Org-Appear shouldn't even mess with the Entites in that case.

@awth13
Copy link
Owner

awth13 commented Apr 1, 2021

Thank you, @SPFabGerman, great work!

Again, not sure why this behaviour happens, but I found how it happens. Because org-appear does not forget the previous element after it was hidden, it rehides it after each command until a new element is encountered. This, somehow, messes with movement. Fixed now.

You work here alerts me, in general, that org-appear affects movement more than it should. Thank you. It's unrelated to this PR, though, so I will do some more testing and merge.

@awth13 awth13 merged commit 1e20d00 into awth13:master Apr 1, 2021
japhir added a commit to japhir/org-appear that referenced this pull request Apr 2, 2021
awth13 pushed a commit that referenced this pull request Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants