[BUG] SIGINT exits leave terminal in dirty state

[anthonyjmartinez]

Describe the bug When sending SIGINT (ctrl-c) to exit ptg the terminal continues to capture mouse input until the reset command is issued.

To Reproduce

1. Install pytermgui
2. Run ptg
3. Exit with ctrl-c
4. Push mouse buttons

Expected behavior Mouse events are not captured.

Screenshots ASCII cast here

System information

$ ptg --version
PyTermGUI v1.1.0
Python: 3.9.2
Platform: Linux-5.10.0-10-amd64-x86_64-with-glibc2.31
Git commit: 47bed90

[bczsalba]

That actually seems to showcase another issue as well, in the window going out of the terminal screen on initial open. I'll look into this early next month when I have a Linux machine on hand, as I'm currently working out of my MacBook.

[anthonyjmartinez]

@bczsalba I think you can close this now. The behavior doesn't exist with v4.0.0.

[bczsalba]

Well I'm certainly glad to hear that! I have no idea what the issue was, what fixed it or when it was fixed but I'm glad it was!

Thank you for the report!

[anthonyjmartinez]

Appears to have resurfaced with the latest release. Even a clean exit keeps capturing mouse clicks.

[bczsalba]

I am beyond weirded out by this issue, happens without a clear reason, disappears the same way and then comes back again. The code that is most likely responsible here (WindowManager.__exit__) is part of a major refactor of its module over at compositor branch, so it might just get randomly fixed again :D.

Will look into it further, just in case.

[bczsalba]

I've checked on my machine, which is running some variation of Ubuntu with i3 and the kitty terminal and couldn't reproduce this error on the new branch, though I didn't try it on master.

Could you check that out?

[anthonyjmartinez]

It may take a few days before I'm able to take a look. I should note that I saw this even when WindowManager and the overall application exits with a clean status.

I'll look at the __exit__ handlers as well and let you know if I see anything that jumps out at me.

[bczsalba]

That's alright! I think what's happening is the with mouse_handler context isn't being properly exited, but I have no clue why.

[bczsalba]

Hey!

Have you been able to find anything? The compositor branch is now part of the latest release.

[anthonyjmartinez]

Sorry for the delay. I've tried with the 5.0.0 release and launching ptg followed by killing it with ctrl-c still results in my terminal needing to be reset lest mouse actions be captured.

(venv) amartinez@scandium:~/Projects/learning/tui$ ptg --version
fatal: Needed a single revision
PyTermGUI v5.0.0

Python: 3.9.2
Platform: Linux-5.10.0-13-amd64-x86_64-with-glibc2.31

[bczsalba]

This issue continues to bug me a lot, knowing nothing of its background. I think I've asked before but I'll do it again since it might have been in a different issue, which terminal is this behaviour exhibited on? There have been some issue with st and other minimal/modern terminals, as they didn't support the full xterm standard.

[anthonyjmartinez]

This is in GNOME Terminal:

amartinez@scandium:~$ gnome-terminal --version
# GNOME Terminal 3.38.3 using VTE 0.66.1 +BIDI +GNUTLS +ICU +SYSTEMD

It doesn't happen in xterm or kitty so I guess one could elect to ignore whatever the root is, but I'll leave that to you. Probably worth an upstream bug report to zero in on the issue.

[bczsalba]

Do other VTE-based emulators, such as Termite, have the same problem?

[anthonyjmartinez]

It sure does. Able to reproduce in Termite.

[bczsalba]

I guess it might be a VTE issue then! Probably could be worked around on my side, but not quite sure if I can as long as I don't understand the problem.

Would an upstream (towards VTE) issue be worth it, what do you think?

[anthonyjmartinez]

I think it would be a good thing to ask about in an issue/question to the VTE maintainers. The bug should be fixed wherever the bug actually exists, but I don't know that it's possible to determine that without asking all sides.

[nikolaevigor]

I am experiencing similar issue, but terminal prints symbols on mouse movement

[bczsalba]

Just checking in, is this still an issue? I tried to see if the problem still happens but I couldn't figure out a way to get any VTE based terminals working on my current system.

[btalb]

We are able to reproduce this relatively consistently across most machines in our company. Standard setups: Ubuntu 20.04, gnome terminal.

What always fixes this for us is ensuring our scripts run print("\x1b[?1002;1006l", flush=True) before exiting (i.e. functionally the equivalent of report_mouse(press_hold, decimal_xterm in pytermgui.ansi_interface).

That says to me the issue is definitely report_mouse() not being called to cleanup. Without digging to deeply, tracing the WindowManager.stop() codepath and __exit__() fns, I can't see where report_mouse() is expected to be being called?

But what does seem to be apparent is the CTRL-C exit path doesn't work by default (some weird capturing of that signal seems to be done on the manager side where it's not processed until ENTER is pressed?).

[bczsalba]

@btalb Thank you, this bit is super useful! The mouse_handler context manager should do the cleanup, but I guess there might be some edge-case where it doesn't. The more confusing part is that it doesn't occur on a lot of systems (anything that isn't vte-based AFAIK), so I don't know if it's Python-side.

Specifically, the relevant code would be:

https://github.com/bczsalba/pytermgui/blob/b287562d65a31dd4a6105cb684abaec61377f820/pytermgui/context_managers.py#L147-L149

Could you maybe try inserting something error-raising (like 1/0) to line 148 (so it only executes if the report_mouse line also does)? I've confirmed that it works on my machine.

[btalb]

Well I don't think these results help clear up anything :stuck_out_tongue: , but this is what I found:

1. Add an exception after report_mouse() cleanup is run:

   finally:
       if event is not None:
           report_mouse(event, method=method, stop=True)
   +           raise Exception("I've cleaned up after myself")

2. Run our application
3. Exit with CTRL-C, and see the exception reported:

   File "/usr/local/lib/python3.8/dist-packages/pytermgui/window_manager/manager.py", line 115, in __exit__
   self.run()
   File "/usr/local/lib/python3.8/dist-packages/pytermgui/window_manager/manager.py", line 195, in run
   self._run_input_loop()
   File "/usr/lib/python3.8/contextlib.py", line 131, in __exit__
   self.gen.throw(type, value, traceback)
   File "/usr/local/lib/python3.8/dist-packages/pytermgui/context_managers.py", line 150, in mouse_handler
   raise Exception("I've cleaned up after myself")
   Exception: I've cleaned up after myself

4. Terminal is not in a good state

[bczsalba]

Hmm, I don't really see what the issue could be then. It's even more annoying that it's terminal specific! What happens if you insert the line terminal.flush() above the report mouse call? Maybe it has a problem with partial sequences.

[stewartad]

I was also encountering this bug on the latest release (7.4.0). I noticed that it only occurs when I install the library from PyPI, and not when I install the library from Github. I diffed the files to see if there was anything obvious and found this in context_managers.py

From PyPI:

@contextmanager
def mouse_handler(
    events: list[str], method: str = "decimal_xterm"
) -> Generator[MouseTranslator, None, None]:

    event = None
    try:
        for event in events:
            report_mouse(event, method=method)

        yield lambda code: translate_mouse(code, method=method)

    finally:
        input(event)
        if event is not None:
            report_mouse(event, method=method, stop=True)

From Github:

@contextmanager
def mouse_handler(
    events: list[str], method: str = "decimal_xterm"
) -> Generator[MouseTranslator, None, None]:

    event = None
    try:
        for event in events:
            report_mouse(event, method=method)

        yield lambda code: translate_mouse(code, method=method)

    finally:
        if event is not None:
            report_mouse(event, method=method, stop=True)

The code is blocking on the input(event) call, and if you Ctrl+C here, then the cleanup code below will not run, leaving the terminal dirty, but any other keypress will exit normally.

[bczsalba]

@stewartad Now that's fascinating! Just to confirm, the PyPi version exhibits the issue for you, but the GitHub one doesn't? Out of curiosity, which terminal emulator do you use?

[stewartad]

@bczsalba yes that's correct. This was using Alacritty

[treamology]

@bczsalba I noticed that, in the report_mouse function, the hover event corresponds to control code 1003. According to the XTerm documentation, this enables both hover and button press tracking, while 1002 (press_hold) enables just button press tracking.

However, by default, WindowManager.run enables both press_hold and hover events. Enabling/disabling both of these at the same time seems to cause the dirty terminal state in my case, as when I call run myself only specifying ["hover"] for the mouse_events argument, the issue doesn't appear anymore.

This issue was happening independently of the one @stewartad described-- even when removing the blocking input(event), button presses were still being printed as seen in OP's case, requiring the above to fix.

All of this was in GNOME Terminal 3.44.0.

[bczsalba]

So it seems to be a VTE-specific issue where disabling both events at the same time causes only one of them to fully disable? Now that I look at it both 1002 and 1003 does seem overkill, though I think the docs I saw mentioned 1002 as press/hold and 1003 as hover only. If that's wrong this might be a simple fix.

[bczsalba]

@treamology Could you test out the latest commit? Using just 1003 seems to work fine, though I've never been able to recreate the issue so can't comment on that.

[treamology]

Actually, looking at context_managers.py:148, it seems like the actual problem is that it's referencing the event variable without being inside a for loop. event remains set to the last value in the previous loop ('hover'), which causes press_hold to remain enabled. This seems to be what has the VTE-specific behavior.

Also, I'm not sure where the previous input(event) line came from, as not only does it not make sense for event to be passed to it in that way, it also doesn't seem to be present in any previous commits? Unless I'm just using GitHub's interface wrong...

[bczsalba]

Actually, looking at context_managers.py:148, it seems like the actual problem is that it's referencing the event variable without being inside a for loop. event remains set to the last value in the previous loop ('hover'), which causes press_hold to remain enabled. This seems to be what has the VTE-specific behavior.

Oh my god, I think you've found it! That definitely seems like it would be the culprit. From what I can tell 1003 is enough as a single report setting already, so this problem never should have existed in the first place.

About the input(event), no clue either. Could have been something I accidentally slipped in while testing, could be a user-side change as well (though unlikely). The point is that it shouldn't have ever been there :)

I'll try to patch it out then, and come back with something that could work.

[bczsalba]

@treamology Could you check the latest commit? I fixed the context manager, and temporarily re-added the double reporting to make sure the core of the issue is gone.

[anthonyjmartinez]

@bczsalba that appears to have done the trick!

@treamology thanks for getting to the root of that

This got shoved to a back burner by a series of both unfortunate and fortunate events in my life (deaths in the family, and then births in the same). I do appreciate that the community stepped in to find the root cause. Thanks all.

[treamology]

Can confirm it's fixed by the latest commit!

[bczsalba]

@anthonyjmartinez I'm sorry and happy to hear that! Glad we could figure this out as well. Thanks @treamology for finding this thing, looking back it's quite obvious but I spent far more time than I'm willing to admit staring at those lines, and never ended up seeing it.

[jdferreira]

When is this fix being pushed into PyPI? I just pip install PyTermGUI, which pulled version a version from May 17, which is before the fix was merged.

[bczsalba]

Just now! Sorry about the delay, I'm currently on vacation.

P.S. Make sure you do pip install -U pytermgui to make sure it updates!