dalanicolai / image-roll.el

Virtual scroll display engine for Emacs
80 stars 4 forks source link

Horizontal scrolling gets reset when ivy/helm/etc. popups show #13

Open NightMachinery opened 1 year ago

NightMachinery commented 1 year ago

Horizontal scrolling gets reset when ivy/helm/etc. popups show (e.g., by doing an M-x when one of these packages is installed).

This does not happen on other major modes for me.

Just switching windows does not reset the scroll position, but running a command in the minibuffer does.

I deleted focus-in-hook, focus-out-hook, after-focus-change-function, but it did not help. Any other suspect hooks I should try?

I don't know if this is a problem with this package or another package is interfering.

I am using Doom and Emacs 29.

The issue exists just using pdf-view-mode, without enabling the image roll minor mode.

dalanicolai commented 1 year ago

Thanks for reporting this issue. Indeed, I have not implemented this functionality (I think this can/should not be fixed via hooks). I don't remember how it works exactly, but I remember that the 'vscroll/hscroll' value has to be reapplied after every redisplay. In image-mode this is done via the image-mode-reapply-winprops function. I am almost sure that in 'image-roll' (which uses/extends image-mode functionality) the last form of the image-roll-redisplay takes care of reapplying the vscroll. However, I did not implement the mechanism also for the hscroll. Unfortunately, I can not afford to work on image-roll anymore. But you can try to study the 'mechanism' for the vscroll functionality, and try to add/copy it for the hscroll functionality (it might take quite some time).

If ever I find the luxury (time and money) again, to work on an Emacs document reader, I will probably focus my effort on finishing the doc-tools package (which works quite well for reading documents already, but not yet for 'editing'). In that case, I will also make sure to implement the mechanism for the hscroll also.

NightMachinery commented 1 year ago

Thanks for reporting this issue. Indeed, I have not implemented this functionality (I think this can/should not be fixed via hooks). I don't remember how it works exactly, but I remember that the 'vscroll/hscroll' value has to be reapplied after every redisplay. In image-mode this is done via the image-mode-reapply-winprops function. I am almost sure that in 'image-roll' (which uses/extends image-mode functionality) the last form of the image-roll-redisplay takes care of reapplying the vscroll. However, I did not implement the mechanism also for the hscroll. Unfortunately, I can not afford to work on image-roll anymore. But you can try to study the 'mechanism' for the vscroll functionality, and try to add/copy it for the hscroll functionality (it might take quite some time).

If ever I find the luxury (time and money) again, to work on an Emacs document reader, I will probably focus my effort on finishing the doc-tools package (which works quite well for reading documents already, but not yet for 'editing'). In that case, I will also make sure to implement the mechanism for the hscroll also.

Thanks!

Does doc-tools currently work? I need imenu, continuous scroll, horizontal scroll and zoom. I don’t need any editing functionality.

dalanicolai commented 1 year ago

Actually, after answering, I just realized that maybe the fix would be really simple (as we can simply use the functionality as implemented by image-mode). And after quickly trying it, the hscroll issue seems fixed now. So I guess the issue will be fixed after you update image-roll.

The doc-tools package works, but zoom and horizontal scroll functionality have not yet been implemented.

NightMachinery commented 1 year ago

Actually, after answering, I just realized that maybe the fix would be really simple (as we can simply use the functionality as implemented by image-mode). And after quickly trying it, the hscroll issue seems fixed now. So I guess the issue will be fixed after you update image-roll.

The doc-tools package works, but zoom and horizontal scroll functionality have not yet been implemented.

winprops in line 378 is not defined:

    (set-window-vscroll (selected-window) (image-mode-window-get 'vscroll winprops) t)

which causes

Debugger entered--Lisp error: (void-variable winprops)
  image-roll-redisplay()
  redisplay_internal\ \(C\ function\)()

This error actually makes the horizontal scroll not reset! I tried replacing the above with

    (non-existent-function-that-errors)
    ;; (set-window-vscroll (selected-window) (image-mode-window-get 'vscroll winprops) t)

And it still worked the same.


Is there a way to activate imenu with image-roll?

Update: imenu-default-create-index-function does the job!

NightMachinery commented 1 year ago

I think I have fixed the issue with this PR.

dalanicolai commented 1 year ago

Ah yes, I see now that I actually copied the wrong (vscroll) line in that 'fix', and indeed also forgot to add the 'let ((winprops ...)(fromimage-mode-reapply-winprops`), very sloppy! Indeed, your code looks more sensible.