Skip to content

Conversation

@kolayne
Copy link
Contributor

@kolayne kolayne commented Oct 14, 2025

Describe your PR, what does it fix/add?

Implements the following behavior:

  • By default, when a tiling window is being focused on a workspace where
    a fullscreen/maximized window exists, respect
    the misc:new_window_takes_over_fs config variable.

  • When a tiling window is focused with the focuswindow dispatcher,
    respect the same config variable, unless it is set to 0 (ignore).
    In that case, the window to be focused takes over fullscreen.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Please, see my review comments on GitHub.

Is it ready for merging, or does it need work?

  1. Did not implement tests yet
  2. Please, see my review comments on GitHub.

Should be merged together with hyprwm/hyprland-wiki#1267

@kolayne kolayne force-pushed the fix_window_focus branch 2 times, most recently from 2db1f1f to 7fa6e12 Compare October 16, 2025 15:26
@kolayne kolayne marked this pull request as ready for review October 17, 2025 00:42
@kolayne kolayne force-pushed the fix_window_focus branch 2 times, most recently from cc0add1 to 2eeb053 Compare October 26, 2025 01:20
@kolayne kolayne force-pushed the fix_window_focus branch 2 times, most recently from 6e75eb1 to 610c6c4 Compare October 26, 2025 02:22
@vaxerski
Copy link
Member

vaxerski commented Nov 5, 2025

I pushed a sizeable refactor to clean up the code in general. LMK if everything still works fine.

@vaxerski
Copy link
Member

should be good now

@vaxerski
Copy link
Member

did I accidentally nuke your fix? Sorry if that happened but yeah tests fail on kitty_activating again xD

@kolayne
Copy link
Contributor Author

kolayne commented Nov 22, 2025

It appears that the execution continues after getFromSocket("/dispatch forcekillactive") before the window is killed. It is confirmed with extra logs that I added, and I can also reproduce this in a regular Hyprland session (if I run hyprctl dispatcher forcekillactive && hyprctl activewindow, the second command outputs the information about the window that was focused before the kill).

Is it a bug?

@vaxerski
Copy link
Member

No, we need to waitUntilWindowsN

Forcekill sends SIGKILL, but it can take a few frames before the window disappears.

@kolayne kolayne force-pushed the fix_window_focus branch 2 times, most recently from 3a8ff55 to 8baea79 Compare November 23, 2025 14:37
… with fullscreen

Renames `misc:new_window_takes_over_fullscreen` into
`misc:on_focus_under_fullscreen` and implements the following behavior:

- By default, when a tiling window is being focused on a workspace where
  a fullscreen/maximized window exists, respect
  the `misc:on_focus_under_fullscreen` config variable.
@kolayne
Copy link
Contributor Author

kolayne commented Nov 23, 2025

Okay, fixed the test. Would you accept a PR renaming the kill* and forcekill* dispatchers to something that describes their functionality more accurately?

@vaxerski
Copy link
Member

probably not? They do what they say...

@vaxerski
Copy link
Member

thanks :)

@vaxerski vaxerski merged commit 40d8fa8 into hyprwm:main Nov 25, 2025
10 checks passed
@kolayne
Copy link
Contributor Author

kolayne commented Nov 26, 2025

(quotes from the wiki)

  • killactive - closes (not kills) the active window

    I would say it does something other than it says in the name

  • forcekillactive - kills the active window

    As someone not well familiar with wayland, I thought that in wayland there were two distinct actions on a window: closing (i.e., asking politely) and killing (i.e., forcing that a window no longer exists). Apparently, what kills do is they send a signal to the process owning the window? I would then call them something like sigkillactive, sigkillwindow, etc. While the new name still says it kills a window, sigkill is a commonly known unix signal name, so it should be more clear that it is an action to a unix process rather than a wayland window.

    Alternatively, the name could be yet more explicit, such as killwindowowner or sigkillwindowowner, but I'm not sure if this is necessary.

    Similar goes for signal, signalwindow dispatchers: I think their names are ok, but the hyprland wiki should probably explain that signals are UNIX signals rather than some wayland concept (is it?).

  • Names are not consistent: dispatchers acting on the active window are called killactive and forcekillactive; dispatchers acting on a window selected by a query are called closewindow and killwindow.

@vaxerski
Copy link
Member

the first one is just a relic and we didnt want to change it to avoid silent config breaks

vaxerski added a commit that referenced this pull request Nov 27, 2025
@jazzpi
Copy link

jazzpi commented Nov 29, 2025

This broke hy3:

$ nix log /nix/store/dibzrsqnjmhlwcdbcvx9hnqfsnsjkl61-hy3-hl0.52.0.drv | grep -A1 error
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/SelectionHook.cpp:13:42: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
   13 |         auto lastWindow = g_pCompositor->m_lastWindow;
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/SelectionHook.cpp:15:32: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
   15 |                 g_pCompositor->m_lastWindow = window->m_self;
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/SelectionHook.cpp:21:32: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
   21 |                 g_pCompositor->m_lastWindow = lastWindow;
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Node.cpp:179:32: error: 'class CCompositor' has no member named 'focusWindow'
  179 |                 g_pCompositor->focusWindow(window);
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Node.cpp:184:32: error: 'class CCompositor' has no member named 'focusWindow'
  184 |                 g_pCompositor->focusWindow(nullptr);
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Node.cpp:221:47: error: 'class CCompositor' has no member named 'focusWindow'
  221 |         if (window != nullptr) g_pCompositor->focusWindow(window);
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/TabGroup.cpp:527:53: error: 'class CCompositor' has no member named 'm_lastMonitor'
  527 |                 auto& last_monitor = g_pCompositor->m_lastMonitor;
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:36:41: error: 'class CCompositor' has no member named 'm_lastMonitor'
   36 |         auto workspace = g_pCompositor->m_lastMonitor->m_activeSpecialWorkspace;
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:37:59: error: 'class CCompositor' has no member named 'm_lastMonitor'
   37 |         if (!valid(workspace)) workspace = g_pCompositor->m_lastMonitor->m_activeWorkspace;
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:146:51: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
  146 |                 auto last_window = g_pCompositor->m_lastWindow;
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:405:58: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
  405 |         auto window = pWindow ? pWindow : g_pCompositor->m_lastWindow.lock();
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:931:46: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
  931 |         auto current_window = g_pCompositor->m_lastWindow.lock();
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:944:48: error: 'class CCompositor' has no member named 'focusWindow'
  944 |                                 g_pCompositor->focusWindow(next_window);
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:977:32: error: 'class CCompositor' has no member named 'setActiveMonitor'
  977 |                 g_pCompositor->setActiveMonitor(next_monitor);
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:1005:32: error: 'class CCompositor' has no member named 'setActiveMonitor'
 1005 |                 g_pCompositor->setActiveMonitor(next_monitor);
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:1016:46: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
 1016 |         auto current_window = g_pCompositor->m_lastWindow.lock();
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:1028:24: error: 'class CCompositor' has no member named 'focusWindow'
 1028 |         g_pCompositor->focusWindow(target);
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:1036:46: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
 1036 |         auto current_window = g_pCompositor->m_lastWindow.lock();
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:1044:66: error: 'class CCompositor' has no member named 'm_lastMonitor'
 1044 |                     this->getWorkspaceFocusedNode(g_pCompositor->m_lastMonitor->m_activeWorkspace.get());
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:1087:46: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
 1087 |         auto focused_window = g_pCompositor->m_lastWindow.lock();
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:1375:43: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
 1375 |         auto last_window = g_pCompositor->m_lastWindow.lock();
--
/build/0r7lm0cfj4dvjl8r3aqrafil1k2mac4q-source/src/Hy3Layout.cpp:1531:64: error: 'class CCompositor' has no member named 'm_lastWindow'; did you mean 'closeWindow'?
 1531 |                 && focused->data.as_window() != g_pCompositor->m_lastWindow.lock()))

@vaxerski
Copy link
Member

great. It needs to be updated.

@Aqa-Ib
Copy link
Contributor

Aqa-Ib commented Dec 12, 2025

This broke exit_window_retains_fullscreen for grouped windows @kolayne. Any hint for fixing it please?

@kolayne
Copy link
Contributor Author

kolayne commented Dec 13, 2025

@Aqa-Ib , most of the refactoring here is by @vaxerski , so I'm not sure 😔

@Aqa-Ib
Copy link
Contributor

Aqa-Ib commented Dec 21, 2025

As this is 1 single commit with multiple and huge changes, it is difficult for me to see where the regression was introduced. Could you send your original patch without vaxry's changes to check it please?

@kolayne
Copy link
Contributor Author

kolayne commented Dec 21, 2025

Yeah, I agree, huge single-commit changes make it much harder to trace things back. I wish we made commits more granular.

Unfortunately, the initial refactoring by vaxry was also pushed as a single commit. Here it is: 16a5f7b (some fixes were applied later)

@Aqa-Ib
Copy link
Contributor

Aqa-Ib commented Dec 21, 2025

I am trying to see your original changes without hyprfucker's huge changes. Are they lost?

@kolayne
Copy link
Contributor Author

kolayne commented Dec 21, 2025

You can use the commit that I shared and take a look at its parent commits. The ones authored by me contain the original changes. But they are probably irrelevant anyway, since the refactoring has rewritten the parts that I was working on (most of them, at least).

Also, why would you use this rude kind of language? I don't think it's appropriate

@Aqa-Ib
Copy link
Contributor

Aqa-Ib commented Dec 21, 2025

I need to joke to not lose my mental sanity, sorry, not trying to be rude though, just for fun.

@Aqa-Ib
Copy link
Contributor

Aqa-Ib commented Dec 21, 2025

#12698

@vaxerski
Copy link
Member

I am trying to see your original changes without hyprfucker's huge changes. Are they lost

hey go fuck yourself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants