search

awth13 / org-appear

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

add 'org-appear-idle-delay' #12

Closed twlz0ne closed 3 years ago

awth13 commented 3 years ago

Thank you for the PR, @twlz0ne!

Would you mind providing a description of what you're hoping to achieve and how?

A couple of thoughts. Wrapping post-cmd in a huge lambda is a non-starter in and of itself -- a more granular approach or, at least, a function call would be preferred -- and it messes with the interaction between org-appear and jit-lock-mode. Even at small values of org-appear-idle-delay, the active element is refontified for a brief moment on each edit. See this issue for details.

The fact that the delay affects both the hiding and unhiding in the same way is not good. There either should be two separate timers or no timer for hiding at all (the hidden parts are disabled as soon as the cursor leaves an element).

twlz0ne commented 3 years ago

@awth13

Would you mind providing a description of what you're hoping to achieve and how?

I think, in the process of moving the cursor quickly, people not very concerned about what's under the cursor. Toggling links or emphasis (especially the former) is rather distracting.

Only when I stay at a point long enough does it mean that I am just interested in the content under the cursor.

The fact that the delay affects both the hiding and unhiding in the same way is not good

For the same reason above. When I accidentally move the cursor out of bounds. I also don't want to see the hiding happen immediately.

A couple of thoughts. Wrapping post-cmd in a huge lambda is a non-starter in and of itself -- a more granular approach or, at least, a function call would be preferred...

Indeed, a huge lambda is not good, but it can be improved in the future.

awth13 commented 3 years ago

Thank you!

When I accidentally move the cursor out of bounds. I also don't want to see the hiding happen immediately.

I'd think that it's more often the case that I move the cursor out of bounds deliberately and not accidentally. In this case, the delay is unnecessary and can even be annoying, depending on your perspective. That's why 2 separate timers is something to consider.

Indeed, a huge lambda is not good, but it can be improved in the future.

I think it might as well be improved now. It shouldn't be too difficult and, what is more important, this commit currently breaks active element editing.

twlz0ne commented 3 years ago

It has been split into 2 separate timers in the latest commit.

awth13 commented 3 years ago

Thank you!

Could you clarify how the code you added to org-appear--show-invisible is relevant to this PR?

Because timers are created inside save-excursion, all fragments that you move across in a quick succession are enabled after the delay. This gets worse after this commit is applied, completely breaking org-appear.

I think the first problem can be solved by moving save-excursion inside the timer. I'm still not sure how to solve another problem I mentioned -- timers break active element editing.

twlz0ne commented 3 years ago

Could you clarify how the code you added to org-appear--show-invisible is relevant to this PR?

To make sure the point still at the element after waiting for N seconds.

I'm still not sure how to solve another problem I mentioned -- timers break active element editing.

Sorry for not doing enough tests. I have fixed this issue in the latest commit.

awth13 commented 3 years ago

To make sure the point still at the element after waiting for N seconds.

May I suggest (when (<= start (point) end) ... instead? Does the same thing but more readable (EDIT: a comment would be nice there too). The same for parent -- you have to grab it with (plist-get elem-at-point :parent) anyway, might as well have just one let.

There is a problem with the way you handled edits -- when you move the cursor quickly, you hit the (equal prev-elem-start current-elem-start) on your second move after entering an element and the timer does nothing. Try a larger value of org-appear-show-delay to see that.

Also, if you exit an element and then return to it without entering another, (equal prev-elem-start current-elem-start) is still true and the timer, again, does nothing -- this one is fixed here though.

awth13 commented 3 years ago

Hey, @twlz0ne!

This is a busy week so it will take time for me to review your progress. I saw this earlier but now it's no longer in your branch tree. I assume you're still working on resolving the issue and I should wait?

I apologise that I'm not much of a help. I want this feature as well but, even as you proposed it, I realised how difficult it would be to implement it within org-appear without relying on hacks. I don't think it's impossible but it will certainly take time to come up with a clean solution. Thank you for your work in looking for one!

Finally, thank you for mentioning the issue in #14. I'll review it this weekend.

awth13 commented 3 years ago

Thanks again, @twlz0ne, for suggesting this idea!

I implemented an idle timer for org-appear using a different approach so I will close this PR.