Mouse-wheel scrolling broken after editing a task externally

[andrei-a-papou]

To reproduce:

1. have several tasks on the list
2. use mouse wheel to scroll up and down and notice how scrolling stops at window boundaries
3. edit a task in external editor (nano)
4. try scrolling now -- and the screen will scroll back beyond window boundaries

Same thing happens if you view a note, for example. Tested in kitty and xfce4-terminal, so doesn't seem to be a terminal issue.

Here's a short video:

[andrei-a-papou]

I don't think pter needs mouse scrolling, can we disable it?

[vonshednob]

pter has mouse scrolling?! That's … new to me! I have no idea what's going on there.

[andrei-a-papou]

Looks like the terminal captures scroll events and sends them to pter. Pter interprets them as up/down arrows, so yes, we have de-facto scrolling, yay :laughing:

Problem is, after you run a command-line editor or viewer and control returns to pter, the UI is not properly reset. If you scroll on the UI, you'll see something like the GIF above :) Does it reproduce on your end?

[vonshednob]

I can reproduce the behaviour. Now only to find out how to fix it one way or the other.

It bugs me a bit that the scroll wheel makes it look like pter actually has mouse support, so I'm even considering to attempt and sabotage the initial "nice" behaviour.

What do you think?

[andrei-a-papou]

Turns out curses can process mouse events, but personally I don't need it in pter. I've tried meddling with https://docs.python.org/3/library/curses.html#curses.mousemask but to no avail.

If initial scrolling behavior could be disabled somehow, I agree it would be desirable. Users wouldn't get any false expectations.

[andrei-a-papou]

I've discovered that sending an escape reset sequence after calling an external program mostly works (I think it still failed for me at least once but I can't reproduce it now). But this is of course very ugly and unreliable, so not a real fix :(

99-weird-scroll-buffer-bug.diff.gz

I duplicated the code because putting it in a method didn't work, no idea why (race condition?). Like I said, ugly.

[andrei-a-papou]

I've just noticed that opening the Help screen can also trigger the bug...

[andrei-a-papou]

OK, resetting the term before paint() seems to work and it also disables scrolling at startup:

99-weird-scroll-buffer-bug.diff.gz

[vonshednob]

Wow, thanks a lot for spending so much of your time!

[andrei-a-papou]

This is not really a fix, more like a workaround, but it "works on my machine" :) The scrolling behavior seems to have something to do with terminfo configuration.

[andrei-a-papou]

At least with the latest git HEAD, applying the above patch often causes the screen to flicker when jump-to action is called in task list. Updated the patch to fix this problem: 99-weird-scroll-buffer-bug.diff.gz

[vonshednob]

Ooof. That’s a heavy handed way to reset the terminal. I suppose the problem is very much a curses and terminal mixture. Let me know when you find a time travel device, so I can go back in time and prevent myself from using curses and go for urwid instead. :grimacing:

[andrei-a-papou]

Yeah, resetting the term is an ugly kludge. I don't know if everyone needs it. Perhaps it could be an option in the config -- to make sure people can toggle it if it causes flickering.

[andrei-a-papou]

OK, I've been using this patch for a while now and here's the summary on my end:

1. I think resetting the term on every paint is probably an overkill. It can also cause unnecessary flickering on some events. It does for me sometimes, but this needs more testing.
2. Resetting the term once at startup is fine and it also kills the initial mouse scrolling. I would just do it.
3. I would reset the terminal when getting control back from external editor, viewer or Help screen -- to work around the most glaring cases.
4. Maybe we should add a reset-term command or just power-up the refresh-screen command? Could be user-configurable (refresh-screen-resets-term = yes or similar). This would give users the ability to trigger a reset manually if necessary.

What do you think?

P. S. It doesn't feel like we can find a nice clean way to deal with various terminfo/terminal/curses weirdness levels to solve this properly any time soon. At least I know I can't :)

[vonshednob]

The worst part is that this mechanism right there? It works for you. It might even work for me (btw, Alt+left and Alt+right don't work for me; I added my termname to the if block, but it doesn’t get there), but who the heck knows how it’ll behave for other terminals.

It’s probably worthwhile to add the additional reset mechanism to the refresh-screen. That function is already a horrible workaround to potential screen output corruption, so it fits right in.

And probably calling it automatically after returning from external programs that surely mess up something (view note, edit task externally; but not when calling any of the hooks) is probably a good idea and should not interrupt the user interface, because it is being passed back to pter anyway.

[vonshednob]

So here goes nothing. In 9e52380a I added the \x1bc escape sequence to all calls that potentially trigger external terminal programs.

The issue with the help screen might persist; I couldn't reproduce that in my configuration. Let me know if that fix works for you or breaks stuff :grimacing:

[andrei-a-papou]

It seems to work OK, but yes, the Help screen messes things up. After control returns from Help screen to task list, scrolling messes up the UI :(

[andrei-a-papou]

btw, Alt+left and Alt+right don't work for me

For me neither, but I don't think pter ever supported these? Or do you mean Ctrl+left/right?

[vonshednob]

Indeed, I meant Ctrl+Left/Right/Del.

So here's a terrible hack to maybe fix the help screen: 3d24fe53.

If that doesn't know: what terminal are you using? I seem to remember that you are using the TERM=xterm work-around?

[andrei-a-papou]

So here's a terrible hack to maybe fix the help screen

Hey, it's not so terrible :) Resetting the term on quit is much better than on every paint :)

Anyway, seems to work OK. I think if pter resets the term on start-up to kill the misleading scrolling behavior, the issue will be mostly "resolved". That, and calling hard_reset_terminal on do_refresh_screen for unforseen situations.

[andrei-a-papou]

I seem to remember that you are using the TERM=xterm work-around?

I set TERM to xterm-256color, but this is to enable color support inside a Docker container. So this is unrelated to the issue under discussion.

[vonshednob]

I think if pter resets the term on start-up to kill the misleading scrolling behavior, the issue will be mostly "resolved". That, and calling hard_reset_terminal on do_refresh_screen for unforseen situations.

Both are in now! I just feel a bit dirty :sweat_smile:

[andrei-a-papou]

refresh-screen works fine, but pter still allows scrolling the screen up (but not down) on startup :(

[vonshednob]

:joy: what an earth is even going on here? If ever there was any question why, by now it should be clear why the package is called curses. I think I gotta go out and sacrifice some cute fluffy animal under a full moon to get this to work!

I'm slowly starting to accept that this will remain weird until in the glorious future a rewrite of pter (or a fork or something) will use a more sane—and less cursed—terminal library.

[andrei-a-papou]

Does it scroll for you on startup (before calling viewer/editor or help screen)?

UPDATE: I also noticed that as of commit 3d24fe5 the same broken scrolling behavior happens on Help screen (scrolling up). And refreshing the Help screen with ^L doesn't seem to reset the term.

[vonshednob]

With the most recent commit I don't get any reaction in pter when using the mouse. Wheel, button, pushing, or tickling. Nothing.

I also tried with env TERM=xterm-256color pter ... and with kitty (both with and without TERM=xterm-256color), to no avail. For me the latest update successfully breaks all mouse-based scrolling.

Regarding the update: so the latest change made it worse? Isn't that just great :unamused:

[andrei-a-papou]

Regarding the update: so the latest change made it worse? Isn't that just great 😒

Yeah, seems that way :( However, if I reset the terminal just before the initial self.paint(), the startup scroll is killed for me: reset-scroll-on-startup.diff.gz

If this works for you too, it's one less problem. The only remaining one would be the Help screen situation.

[vonshednob]

That's interesting -- so when you move the reset to that position it breaks the scrolling successfully?

[andrei-a-papou]

It does. I also "fixed" the Help screen, yay.

But I'm thinking peppering the code with multiple calls to hard_reset_terminal may not be the best approach. I've just discovered that the scrolling comes back when you create a new todo in distraction-free mode :( So another call is needed here.

Perhaps the original patch that reset the main screen on paint was cleaner (if that word even applies here)... Or maybe the distraction-free mode situation is the last hurdle, I don't know. I'll need to use pter for a couple days with the new patches and see.

[andrei-a-papou]

I also "fixed" the Help screen, yay.

For what it's worth:

reset-scrolling-on-help-screen.diff.gz

[vonshednob]

At this point I'm wondering whether it's really worth the efford. Sure, scrolling is weird, but it doesn't actually break the program.

It's not impossible to fix, but is the amount of work worth it?

I'm only half joking when I mention again that I'm considering replacing the backend with urwid (don't get me wrong -- it'd be a huge undertaking though).

[andrei-a-papou]

I guess it depends on the user's habits. Me, I frequently scroll, so it's annoying when the screen slides up unexpectedly :) So it's not the initial scrolling that's really the issue, it's the kind of behavior you can see on the GIF above.

But if a person never scrolls in a TUI app, they might not notice the problem at all.

I've updated my local patch and have been testing it for a couple of days. I'll post it tomorrow, if you don't mind, it needs a little more testing.

If the patch works for you, I would close the issue.

[vonshednob]

I mean, it doesn't break it for me :sweat_smile: I guess that's the best at this point to hope for.

Ie. your patches are in!

[andrei-a-papou]

OK, here's a new version that seems to cover all bases with no visible side-effects. It also fixes the "New task" dialog. I've also made the reset feature configurable (enabled by default).

Can you test it and see if it works for you?

reset-terminal.diff.gz

[vonshednob]

If it's all the same to you, I'd like to set the reset-terminal option to no by default.

Other than that, it appears to work fine for me! So I'd be fine to ship it :)

[andrei-a-papou]

If it's all the same to you, I'd like to set the reset-terminal option to no by default.

Absolutely!

Other than that, it appears to work fine for me! So I'd be fine to ship it :)

Great! I'll keep testing and if I stumble upon a related bug, I'll send another patch!

[vonshednob]

Let's just say this is fixed for now.

[vonshednob]

It's released now! pip update or similar will do the trick.

Again, thank you so much for your help and patience @andrei-a-papou ! It's really appreciated!