io12 / org-fragtog

Automatically toggle Org mode LaTeX fragment previews as the cursor enters and exits them
MIT License
406 stars 14 forks source link

Add org-fragtog-preview-delay #25

Closed awth13 closed 3 years ago

awth13 commented 3 years ago

Purpose Enable delayed preview so that moving through LaTeX fragments does not toggle them unless the cursor stops and Emacs is idle.

Testing Tested by creating multiple LaTeX fragments and moving through them with different org-fragtog-preview-delay values. Everything works as expected. If the cursor leaves a fragment before the timer fired, the timer is cancelled and org-fragtog--enable-frag is not called.

There is one bug related to the way post-cmd works. When the cursor moves through an untoggled fragment from left to right and stops at the next point after the fragment, org-fragtog--cursor-frag still returns the fragment (despite adjusting by :post-blank as a precaution).

This $LaTeX$ fragment
            ^
            |       
     this point considered part of the fragment

This means that the timer is not cancelled if the user stops at that point and the fragment is disabled. This does not happen when the cursor moves from right to left or when the cursor moves through a fragment that has been disabled already.

Further comments I am working on the same feature in org-appear and decided I might as well contribute my solution to org-fragtog. The bug described above does not affect org-fragtog as much as it does org-appear -- in the latter, it can lead to the next immediately adjacent fragment not being toggled at all. I am looking for a solution but so far no ideas. (EDIT: I believe that a solution for this will not affect how the timer creation/cancellation is implemented).

awth13 commented 3 years ago

Update I expected, based on Emacs documentation, that running org-fragtog--disable-frag with zero timer will function in the same way as running it without a timer. This turned out to be not true -- there was a very small delay causing fragments to appear as though the cursor entered them before they are disabled. Accordingly, I added a check in post-cmd that will run org-fragtog--disable-frag without a timer if preview-delay is 0.

Sorry for the slow progress here. I would appreciate any feedback regarding the bounds bug I described earlier -- still puzzled by it.

awth13 commented 3 years ago

The bounds bug is resolved. The reason for it is described here. In short, all functions in post-command-hook run before Emacs adjusts the point to skip over invisible elements. As a result, we get an "incorrect" point when we are inside org-fragtog--post-cmd. My solution is to make org-fragtog--disable-frag re-evaluate the frag-at-point (calling org-fragtog--cursor-frag) when it runs with a timer -- this way it gets the adjusted point. Not the most elegant but, as far as I can tell, post-command point adjustment is a low-level thing we don't really have access to.

There is another issue I discovered. When we want to edit a disabled fragment by extending it, the fragment briefly stops being a LaTeX fragment because there is whitespace before the enclosing $. This is just an inconvenience when org-fragtog runs without a delay -- even though org-fragtog--enable-frag is called unnecessarily, everything is fine once we add whatever we wanted to add. With a delay, as currently implemented, it becomes a problem since a new timer is started for the LaTeX fragment once it is extended. If we now leave the fragment before the timer fired, the fragment will not be enabled. This issue does not affect \(...\) and \[...\] fragments.

$2 + 2 $
      ^
      |
   Not a LaTeX fragment once this space added

Of course, a simple solution is to remove the check that prevents org-fragtog--enable-frag from running if there is an active timer (if we left the fragment before the timer fired). I will think more on this, however, because I think that running org-fragtog--enable-frag unnecessarily is wasteful. Again, any feedback is appreciated.

io12 commented 3 years ago

Thanks for the PR! And sorry for not responding earlier.

I like this feature and want to merge it in eventually, after fixing the org-fragtog--enable-frag issue. It might be possible to just check if the fragment still exists and only enable it if it does.

Also, it's an interesting coincidence that this PR has the exact same name as https://github.com/io12/org-fragtog/pull/14. I think that one started the timer when a cursor left a fragment instead of entering one though, so they aren't identical.

Also thanks for developing org-appear. It's very useful.

io12 commented 3 years ago

There is another issue I discovered. When we want to edit a disabled fragment by extending it, the fragment briefly stops being a LaTeX fragment because there is whitespace before the enclosing $.

This should be fixed now in https://github.com/io12/org-fragtog/commit/156da4ae5a64a42d20123acaa3aa5297b9d44f01.

awth13 commented 3 years ago

This should be fixed now in 156da4a.

Thanks! I will merge it into the PR and test on the weekend, hopefully.

Also thanks for developing org-appear. It's very useful.

The credit really goes to you -- org-appear started as pretty much a copy of org-fragtog with a twist. It was even called org-emphtog during the first stages of development :)

Of course, I've been browsing this repo and issues/PRs when working on org-appear, which might explain the similarity to #14. I didn't mean to "cite" it verbatim though, probably subconscious.

awth13 commented 3 years ago

With a delay, as currently implemented, it becomes a problem since a new timer is started for the LaTeX fragment once it is extended. If we now leave the fragment before the timer fired, the fragment will not be enabled.

Marking this as a draft as the above issue is still blocking. I thought it would be possible to resolve by keeping track of the last visited point and disabling without a timer if the last visited point is within the cursor-frag (meaning it's the same frag but edited). However, it runs into the same problem of post-command-hook functions receiving the unadjusted point because adjustment happens after them.

I continue working on that but, since I can't see any good solutions at the moment, I'm not sure when or if I will be able to resolve it.

awth13 commented 3 years ago

The enabling issue is resolved by changing the way the enable decision works. Previously (2acc34b), we checked whether a timer still exists -- if it does, then the frag-at-prev-pos was never disabled and there's nothing to enable. This is problematic when it comes to new fragments or edited $ ... $ fragments.

01a60e5 implements a superior way. Now, we check if the frag-at-prev-pos has an (LaTeX image) overlay over it. If yes, then it was not disabled and we skip enabling it. If not, we enable it regardless of an existing timer. I assumed that there cannot be other overlays within LaTeX fragments -- if this is not true, it is possible to be more specific and check that (overlay-get this-overlay 'org-overlay-type) returns the correct type (org-latex-overlay).

The proposed delay functionality now works as expected. I also checked that there are no tabs left in the commits. The PR is ready for review.

io12 commented 3 years ago

Sorry for the delay. This looks great thanks!