X.A.DynamicProjects removes current static workspace if empty when switching to a project

jackroi commented 2 months ago

Problem Description

When switching to a project (from XMonad.Actions.DynamicProjects) from a static workspace (ie. defined in xmonad.hs) that contains no windows, the initial workspace gets removed.

Example: switchProject tempProject while focus is on workspace 1. Workspaces: before -> after Actual: [*1,2,3] -> [2,3,*TMP] Expected: [*1,2,3] -> [1,2,3,*TMP]

`switchProject` semantic

The function switchProject :: Project -> X () is supposed to create and focus a dynamic workspace, and run the associated start-up hook. By inspecting the source code, I found the following:

https://github.com/xmonad/xmonad-contrib/blob/b3c249434d7482eb9dee236b150f7a8fba0380ce/XMonad/Actions/DynamicProjects.hs#L265-L266

that clearly indicates the will of the author to delete the focused project when it is empty (however this is neither documented nor working). Then in another part of the code:

https://github.com/xmonad/xmonad-contrib/blob/b3c249434d7482eb9dee236b150f7a8fba0380ce/XMonad/Actions/DynamicProjects.hs#L218-L220

suggesting that he didn't want that functionality.

Moreover there are 2 related commits: The first (Dec, 2015) is exactly about this problem, and the commit message says:

Don't auto-delete workspaces: Previously I was removing empty workspaces after switching away from them, but this seemed to cause a bug when workspaces were defined statically in your XMonad configuration.

The [second]() (Nov 2016) mentions in the changelog:

Switching away from a dynamic project that contains no windows automatically deletes that project's workspace. The project itself was already being deleted, this just deletes the workspace created for it as well.

The bug

The incriminated code is this: https://github.com/xmonad/xmonad-contrib/blob/b3c249434d7482eb9dee236b150f7a8fba0380ce/XMonad/Actions/DynamicProjects.hs#L256-L271

Given the previous context, this part of the function: https://github.com/xmonad/xmonad-contrib/blob/b3c249434d7482eb9dee236b150f7a8fba0380ce/XMonad/Actions/DynamicProjects.hs#L267-L269 is either not wanted at all (don't really want to remove a project when switching away from it, even if it is empty), or if the project deletion is wanted, it is bugged, in fact a "user created project" (will come back to this later) has always the field projectStartHook = Just, due to dynamicProjectsStartupHook function: https://github.com/xmonad/xmonad-contrib/blob/b3c249434d7482eb9dee236b150f7a8fba0380ce/XMonad/Actions/DynamicProjects.hs#L205-L224 where in the last lines, if a project has projectStartHook = Nothing it is forced to projectStartHook = Just (return ()).

Line 260 of switchProject uses the function currentProject to obtain current project: https://github.com/xmonad/xmonad-contrib/blob/b3c249434d7482eb9dee236b150f7a8fba0380ce/XMonad/Actions/DynamicProjects.hs#L232-L238 https://github.com/xmonad/xmonad-contrib/blob/b3c249434d7482eb9dee236b150f7a8fba0380ce/XMonad/Actions/DynamicProjects.hs#L357-L359 This function always return a Project, even for standard workspaces, with the difference that projectStartHook = Nothing (this is a sort of fake project that can never be created by a user). So oldp will contain either:

a real project (created in some way by the user) (but with projectStartHook never equal to Nothing (Nothing is substituted with Just (return ())), when the current workspace is a project;
or an artificial default project with projectStartHook = Nothing, when the current workspace is not a project.

So the check isNothing ... https://github.com/xmonad/xmonad-contrib/blob/b3c249434d7482eb9dee236b150f7a8fba0380ce/XMonad/Actions/DynamicProjects.hs#L267 will always return true for non projects, and never return true for project, that is the opposite of what we want.

Possible solutions

Solution 1

Delete this snippet of code: https://github.com/xmonad/xmonad-contrib/blob/b3c249434d7482eb9dee236b150f7a8fba0380ce/XMonad/Actions/DynamicProjects.hs#L267-L269
Provide a function to manually delete projects (and associated workspace)

Solution 2

Change isNothing with isJust

Solution 3

Avoid overwriting projectStartHook with a dummy Just (return ()) in dynamicProjectsStartupHook
Make currentProject return a Maybe, instead of building a fake Project.
Substitute isNothing (projectStartHook oldp) with isJust oldP

This is however a breaking change since currentProject :: X Project is exported. In any case, I think it should be documented that it returns a project with projectStartHook = Nothing.

Deleting projects

Finally, provide a way to delete projects manually. After all, deleting projects only when switching to other projects is not a sufficient way to close all the projects.

Steps to Reproduce

Focus a static workspace that contains no windows.
Switch to a project, either with switchProject or switchProjectPrompt.
The initial workspace gets deleted.

Configuration File

import XMonad
import XMonad.Util.EZConfig
import XMonad.Hooks.EwmhDesktops
import qualified XMonad.StackSet as W

import XMonad.Actions.DynamicProjects
import XMonad.Actions.DynamicWorkspaces

-- Xmobar
import XMonad.Hooks.DynamicLog
import XMonad.Hooks.StatusBar
import XMonad.Hooks.StatusBar.PP
import XMonad.Hooks.ManageDocks
import XMonad.Util.Loggers

-- Terminal
myTerminal = "alacritty"

-- Static workspaces
myWorkspaces = [ "1", "2", "3", "4", "5", "6", "7", "8", "9" ]

--------------------------------------------------------------------------------
-- Dynamic projects

tempProject :: Project
tempProject = Project { projectName          = "TMP"
                          , projectDirectory = "~/"
                          , projectStartHook = Just $ spawn myTerminal
                          }

projects :: [Project]
projects = [ tempProject
           ]
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
-- Statusbar
mySB = statusBarProp "xmobar" (pure myXmobarPP)

myXmobarPP :: PP
myXmobarPP = def
    { ppCurrent         = magenta . wrap "[" "]"
    , ppVisible         = purple . wrap "[" "]"
    , ppHidden          = blue . wrap "" ""
    , ppHiddenNoWindows = lowWhite . wrap "" ""
    , ppUrgent          = red . wrap (yellow "!") (yellow "!")
    , ppSep             = magenta " • "
    , ppTitleSanitize   = xmobarStrip
    , ppOrder           = \[ws, l, _] -> [ws, l]
    }
  where
    blue, lowWhite, magenta, purple, red, white, yellow :: String -> String
    magenta  = xmobarColor "#ff79c6" ""
    purple   = xmobarColor "#bd93f9" ""
    blue     = xmobarColor "#41a6b5" ""
    white    = xmobarColor "#f8f8f2" ""
    yellow   = xmobarColor "#f1fa8c" ""
    red      = xmobarColor "#ff5555" ""
    lowWhite = xmobarColor "#bbbbbb" ""
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
-- Keybindings

myKeys = [ ("M-y", switchProject tempProject)
         ]
--------------------------------------------------------------------------------

myConfig = def
    { modMask = mod4Mask        -- Super key
    , terminal = myTerminal
    , workspaces = myWorkspaces
    }
  `additionalKeysP` myKeys

main :: IO ()
main = xmonad
     . ewmhFullscreen
     . ewmh
     . withSB mySB
     . dynamicProjects projects
     . docks
     $ myConfig

Checklist

[x] I've read CONTRIBUTING.md
I tested my configuration
- [x] With xmonad version 0.18.0-14
- [x] With xmonad-contrib version 0.18.0-14

slotThe commented 2 months ago

Solution 2

Change isNothing with isJust

I'd imagine the code is just an unfortunate typo, making this the safest option.

I find "delete empty workspaces upon switching off of them" to be quite surprising behaviour, though. Perhaps people who use the module find this convenient? Since the behaviour seems to have been broken for close to 10 years, maybe not so much :)

geekosaur commented 2 months ago

With any contrib that old, the real question is whether anyone has used it since it was added. 😁

jackroi commented 2 months ago

Yes, solution 2 is the safest. However, I would prefer solution 1, since as you say it's a surprising behaviour, and I would prefer to delete a project when I consider it ended, and not when it happens to have no windows. And after all, I don't expect no one to rely on this behaviour since it is an old bug and it's not documented.

With any contrib that old, the real question is whether anyone has used it since it was added. 😁

I switched to XMonad recently, and I was really happy when I discovered this module, since it does exactly what I need.

slotThe commented 2 months ago

Yes, solution 2 is the safest. However, I would prefer solution 1, since as you say it's a surprising behaviour, and I would prefer to delete a project when I consider it ended, and not when it happens to have no windows. And after all, I don't expect no one to rely on this behaviour since it is an old bug and it's not documented.

That's fine with me; do you want to prepare a PR?

jackroi commented 2 months ago

Yes, I will try later today.

xmonad / xmonad-contrib