raboof / notion

Tiling tabbed window manager
https://notionwm.net/
GNU Lesser General Public License v2.1
268 stars 63 forks source link

Intermittent bug in notion 4: META-Return (full-screen) sometimes ignored #316

Closed dkogan closed 1 year ago

dkogan commented 3 years ago

Hi. I recently moved to notion-4 fulltime, and it's 99% functional (thanks tons!) but periodically I see issues.

I haven't debugged this yet, but maybe just the description will speak to you. I have META-Return bound to toggle the full-screen state of a window:

defbindings("WGroupCW", {
    bdoc("Toggle client window group full-screen mode"),
    kpress(META.."Return", "WGroup.set_fullscreen(_, 'toggle')"),
})

This works, and I've been using it for at least 10 years. With notion-4 I sometimes see existing emacs windows stop being able to react to META-Return. All of a sudden notion stops interpreting that key combination, and passes the keystroke to the application (emacs in this case), and emacs then tells me it doesn't know what to do with it. Which is right: it wasn't supposed to see it; notion was supposed to handle it upstream.

So far I've seen this happen only to emacs and only to META-Return. Other key combinations continue to work. Other emacs windows (same executable) work. Killing the broken window, and bringing up a new one is the only work around I've found so far. Obvious suspects?

dkogan commented 3 years ago

Hi. I'm not very familiar with how Xkb works. Can I get some debugging suggestions for this? Can you point me to the code that decides where each keystroke is interpreted? Thanks.

dkogan commented 3 years ago

I found a reproducing sequence for this bug. It's not just emacs: any window is affected. Furthermore, this revealed another behavior change from notion 3 that we should track down, and revert.

Recipe:

Let's say we have a workspace split vertically into two frames, with each frame containing one window:

---------------------
|         |         |
|         |         |
|         |         |
| Frame 1 | Frame 2 |
|  Win 1  |  Win 2  |
|         |         |
|         |         |
--------------------

The windows could be anything. While Frame 1 (and window 1) are active, I unsplit the workspace. With my config (emacs-like bindings from the old notion-scripts) this is done with META..x..0:

    submap(META.."x", {
        bdoc("Destroy current frame."),
        kpress ("AnyModifier+0", "WTiling.unsplit_at(_, _sub)"),
        ...
    }

With Notion3, the workspace now has one big frame, with window 1 still being active.

With Notion4, the workspace also has one big frame, but window 2 is now active. And when I switch back to window 1, I can't full-screen it with META..Return, as reported in the bug report originally.

I'm trying a bisection now.

dkogan commented 3 years ago

OK, I give up on the bisecting: it broke when chunks of this were in submodules, but the repositories that those submodules point to are gone.

raboof commented 3 years ago

I found a reproducing sequence for this bug

That's great! I can reproduce it on a regular configuration as well! (unsplitting is META+X there)

Furthermore, this revealed another behavior change from notion 3 that we should track down, and revert (...) I'm trying a bisection now.

IMO we shouldn't look at notion3 too much anymore: aside from the submodules making things complicated, there are quite some differences due to not including the Ion-licensed history in Notion4, which not only makes things hard to track down, but if we 'find' things in the Ion-licensed code we can't really use them anyway... let's just diagnose and fix this problem staying within Notion4-land ;).

2 other things that would be useful to try come to mind though:

dkogan commented 3 years ago

Yeah. We shouldn't "revert' the changes, but a successful bisection could make the bug easier to track down.

The bug exists even without mod_dock and mod_statusbar.

The issue originates prior to 4.0.0. I see it here: https://github.com/raboof/notion/commit/3bd0378c which is where all the complicated branching starts, as you look back in time.

dkogan commented 3 years ago

Even older, the bug exists here: 40b581165f04a5c1dfca3d33856a1c8524ab822f