xmonad / xmonad-contrib

Contributed modules for xmonad
https://xmonad.org
BSD 3-Clause "New" or "Revised" License
584 stars 274 forks source link

Terminal with noborder doesn't come back after closing a swallowed app #638

Closed syunsuke closed 2 years ago

syunsuke commented 2 years ago

Hi everyone.

I thought I found a problem. So I report the problem.

I've read the CONTRIBUTING.md But I've never issued. so The way to issue may be wrong. If so, would you tell me?

Problem Description

When I use smartBorders of XMonad.Layout.NoBorders module and I set XMonad.Hooks.WindowSwallowing hook in my xmonad.hs, I open "a" terminal and then call "chromium" on the terminal, so chromium opens well.

But I close the chromium, workspace is empty!!

I think the terminal which called the chromium should come back, but it goes away.

And acutually if I don't use smartBorders nor noborder, the terminal comes back.

Steps to Reproduce

I set smartborder and windowswallowing in my xmonad.hs

I opened one kitty terminal

I issued "chromium" on the terminal. (or "nigrogen" my favorit wallpaper seting app)

The gui app was opened.

I closed the gui app.

That workspace was empty.

Configuration File

Please include the smallest full configuration file that reproduces the problem you are experiencing:

import XMonad
import XMonad.Layout.NoBorders
import XMonad.Hooks.WindowSwallowing

main = do
  xmonad 
    $ def { terminal          = "kitty"

          , handleEventHook   = myHandleEventHook
                                <+> handleEventHook def

          , layoutHook        = smartBorders $ Tall 1 0.03 0.5
          -- , layoutHook        = Tall 1 0.03 0.5
          }

---------------------------------------------------------------
-- WindowSwallowing
---------------------------------------------------------------
myHandleEventHook 
  = swallowEventHook (className =? "kitty" 
                 <||> className =? "Alacritty"
                 <||> className =? "XTerm") (return True)

Checklist

slotThe commented 2 years ago

I think this is due to the fact that ConfigureEvents are not guaranteed to "preserve" the state of our stack in any way. The code acknowledges this but still checks whether the stored stack is a Just value; i.e., has at least one window in it. Since changing the window border (which is what smartBorders does) also produces ConfigureEvents, this creates some friction.

@syunsuke could you try the following patch and see if that works for you?

diff --git a/XMonad/Hooks/WindowSwallowing.hs b/XMonad/Hooks/WindowSwallowing.hs
index 4af1abb4..76f9c8b4 100644
--- a/XMonad/Hooks/WindowSwallowing.hs
+++ b/XMonad/Hooks/WindowSwallowing.hs
@@ -1,4 +1,5 @@
 {-# LANGUAGE NamedFieldPuns #-}
+{-# LANGUAGE LambdaCase #-}
 -----------------------------------------------------------------------------
 -- |
 -- Module      :  XMonad.Hooks.WindowSwallowing
@@ -155,7 +156,7 @@ swallowEventHook parentQ childQ event = do
         maybeOldStack        <- XS.gets stackBeforeWindowClosing
         oldFloating          <- XS.gets floatingBeforeClosing
         case (maybeSwallowedParent, maybeOldStack) of
-          (Just parent, Just oldStack) -> do
+          (Just parent, oldStack) -> do
             -- If there actually is a corresponding swallowed parent window for this window,
             -- we will try to restore it.
             -- because there are some cases where the stack-state is not stored correctly in the ConfigureEvent hook,
@@ -163,29 +164,32 @@ swallowEventHook parentQ childQ event = do
             -- if it is, we can restore the parent exactly where the child window was before being closed
             -- if the stored stack-state is invalid however, we still restore the window
             -- by just inserting it as the focused window in the stack.
-            stackStoredCorrectly <- do
-              curStack <- withWindowSet (return . currentStack)
-              let oldLen = length (W.integrate oldStack)
-              let curLen = length (W.integrate' curStack)
-              return (oldLen - 1 == curLen && childWindow == W.focus oldStack)
-
-            if stackStoredCorrectly
-              then windows
+            stackStoredCorrectly childWindow oldStack >>= \case
+              Nothing -> windows (insertIntoStack parent)
+              Just os -> windows
                 (\ws ->
-                  updateCurrentStack
-                      (const $ Just $ oldStack { W.focus = parent })
-                    $ moveFloatingState childWindow parent
-                    $ ws { W.floating = oldFloating }
-                )
-              else windows (insertIntoStack parent)
+                   updateCurrentStack
+                       (const $ Just $ os { W.focus = parent })
+                     $ moveFloatingState childWindow parent
+                     $ ws { W.floating = oldFloating })
             -- after restoring, we remove the information about the swallowing from the state.
             XS.modify $ removeSwallowed childWindow
             XS.modify $ setStackBeforeWindowClosing Nothing
           _ -> return ()
-        return ()
     _ -> return ()
   return $ All True

+stackStoredCorrectly :: Window -> Maybe (W.Stack Window) -> X (Maybe (W.Stack Window))
+stackStoredCorrectly childWindow = \case
+  Nothing       -> return Nothing
+  Just oldStack -> do
+    curStack <- withWindowSet (return . currentStack)
+    let oldLen = length (W.integrate oldStack)
+    let curLen = length (W.integrate' curStack)
+    return $ if   oldLen - 1 == curLen && childWindow == W.focus oldStack
+             then Just oldStack
+             else Nothing
+
 -- | insert a window as focused into the current stack, moving the previously focused window down the stack
 insertIntoStack :: a -> W.StackSet i l a sid sd -> W.StackSet i l a sid sd
 insertIntoStack win = W.modify
syunsuke commented 2 years ago

Hi, slotThe.

I tried the patch. And the patched code works very well. Now my kitty terminal comes back.

Thank you, so much.