Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

X.L.PerWorkspace possible bug #76

Closed
davama opened this issue Aug 24, 2016 · 32 comments
Closed

X.L.PerWorkspace possible bug #76

davama opened this issue Aug 24, 2016 · 32 comments

Comments

@davama
Copy link
Contributor

davama commented Aug 24, 2016

Hello,
Had this issue on xmonad mailing list here
Not sure really if it's a PerWorkspace bug but that was the final conclusion.

So basically to summarize what happened on that chain. I was having issues with avoidStruts under git-xmonad but nothing of that sort using haskell-xmonad (using cabal install).

Mr. Bogdan suggested to remove PerWorkspace all together, which i did but no change. (changes here) (currently using PerWorkSpace under haskell-xmonad; willing to test again)

When i use haskell-xmonad avoidstruts and ToggleStruts seems to work fine. Under git-xmonad they do not. My dzen bars are covered by the window clients on all WS (visible, non-visible).

My config is here.

Decided to bring this issue to GIT since it's much easier to read and maybe more eyes will look at it.

Hope this is clear.

Thanks,
Dave

@geekosaur
Copy link
Contributor

What problems were you having, exactly?

Removing PerWorkspace pretty much rules it out as a cause, so I don't quite see how it could have been concluded that it was at fault. ManageDocks (which provides avoidStruts) did change in 0.12, and there's at least one significant bug in it that is not in git.

I also note that your "remove PerWorkspace" version unconditionally applies avoidStruts to all layouts including the fullscreen layout; if you were expecting it to not apply there (I can't tell since you didn't say what issues you were having), reconsider where you are putting avoidStruts.

@davama
Copy link
Contributor Author

davama commented Aug 24, 2016

Basically when under git-xmonad my dzen bars are covered by the window clients and ToggleStruts no longer works. This does not happen when using haskell-xmonad (bars are respected and togglestruts works).

I had gone through a whole session on the mailing list trying to figure out what it was exactly (back in March) and that's where things were left. (PerWorkspace being the issue)

My current xmonad.hs file uses PerWorkspace which is in my profile. (link on first post)
my current layouts looks like so:

myLayoutHook =  onWorkspace (myWorkspaces !! 7) (avoidStruts (gimpLayout))  -- for gimp
        $ onWorkspace (myWorkspaces !! 8) (avoidStruts (imLayout))  -- for pidgin
        $ avoidStruts (standardLayouts) ||| fullscreen

    where
        standardLayouts = named "+" (spacing myBorderSpace $ Grid) -- grid
                ||| named "[]-" (spacing 1 $ Tall 1 (3/100) (1/2))  -- tall
                ||| named "TT"  (spacing myBorderSpace $ Mirror (Tall 1 (3/100) (1/2))) -- mirror tall
        fullscreen  = named "[_]" (noBorders (fullscreenFull Full)) -- full
        imLayout    = withIM (18/100) (Title "Buddy List") Grid -- actual name "IM Grid"
        gimpLayout  = withIM (0.130) (Role "gimp-toolbox") $ reflectHoriz $ withIM (0.2) (Role "gimp-dock") Grid    -- actual name "IM ReflectX IM Grid"

Thanks!

@f1u77y
Copy link
Contributor

f1u77y commented Aug 25, 2016

dzen2 doesn't set _NET_WM_WINDOW_TYPE, that's why it's not handled by functions in X.H.ManageDocks
UPD: looked through dzen docs, do you use -dock option? for me your config with it(and without PerWorkspace works fine

@davama
Copy link
Contributor Author

davama commented Aug 25, 2016

@f1u77y thanks for the comment.
Google led me to this

So i changed my scripts calling dzen2 with dzen2 -dock and now docks/bars are respected and togglestruts also work.

Not sure why it works now. Tested with both haskell-xmonad and git-xmonad. (never had an issue with haskell-xmonad)

Did dzen behavior change or xmonad change?
Thanks

@geekosaur
Copy link
Contributor

On Thu, Aug 25, 2016 at 6:09 PM, Dave [email protected] wrote:

Not sure why it works now. Tested with both haskell-xmonad and git-xmonad.
(never had an issue with haskell-xmonad)

Before strut caching was added, xmonad would iterate through all windows to
determine which ones had struts set every time it ran a layout. This is
prohibitively expensive with large numbers of windows.

xmonad 0.12 caches existence of struts, but does so buggily. It was
completely rewritten, but one side effect is that struts are only ever
checked for if a window is _NET_WM_WINDOW_TYPE_DOCK. Strictly speaking this
is incorrect because the spec does not explicitly say struts should only be
applied to dock and desktop windows, although strictly speaking clients
that set struts on non-dock windows are non-compliant because such windows
should be considered floating and user positionable, not attached to a
screen edge and fixed location. So the question is whose bug is more
incorrect. :)

As a practical matter, having to check every new window for struts is
somewhat expensive since the check needs to be done twice (if
_NET_WINDOW_STRUT_PARTIAL is not found, we need to look for
_NET_WINDOW_STRUT for backward compatibility). Older xmonad's check was
even more expensive, though, since it happened every time the layout is run
instead of once per created window.

brandon s allbery kf8nh sine nomine associates
[email protected] [email protected]
unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

@f1u77y
Copy link
Contributor

f1u77y commented Aug 25, 2016

@geekosaur yes that check is more expensive then checking only _NET_WM_WINDOW_TYPE but there can be many other checks in manageHook and new windows appear rarely in normal situation. i think checkDock should be rewritten to match ewmh(never payed attention on it before)

about the original issue: i know situation when it reproduces

  1. something sends message to all layouts, including those for which runLayout have never been called
  2. these layouts (PerWorkspace or IfMax(see Display bug with IfMax, PerScreen, PerWorkspace with decorated layouts  #75 which is, i think, stongly connected to this issue)) pass that message to wrong sub-layout
  3. BUG!
    but i really don't know what's the best option to fix this

@geekosaur
Copy link
Contributor

Not sure it's those specifically. I realized last night that I have more evidence that some things are being run at the wrong time and/or in the wrong context, meaning any conditional layouts etc. are likely to fail in unexpected ways.

While testing/debugging the NSP loggers, I had discovered that the logHook is run at least 3 times during xmonad startup, before the layout has been run properly. Last night, I realized this was very likely related to both these bugs and the particular way that the 0.12 strut bug manifests, which has otherwise been obscure: the layout is run at least once (which triggers the logHook to be run), and likely multiple times, before any windows have been recognized and possibly before the monitor information and other aspects of server and display configuration have been processed.

@f1u77y
Copy link
Contributor

f1u77y commented Aug 26, 2016

@geekosaur but WM_NAME change runs logHook too. so, it can be run many times during startup(if some applications are starting)

and i still dont know what to do with layouts that require to be run for updating their state. i see one of soltions(sending some message that makes layout update on startup for PerWorkspace and on any windowSet change for IfMax)) but im not sure if that's good solution or just an ugly workaround

@liskin
Copy link
Member

liskin commented Sep 1, 2016

@davama I had to do this to make avoidStruts work again: liskin/liskin-xmonad-conf@b1154d2, liskin/liskin-xmonad-conf@6660fa9
Can you try something similar and see if it works for you?

I suspect the problem is that AvoidStruts caches the list of struts in its layout modifier instance and updates it via messages, but these messages only ever reach it if it's the current active layout. I have described my reasoning and my proposed fix here: 28e9f8b

@f1u77y
Copy link
Contributor

f1u77y commented Sep 1, 2016

@liskin

AvoidStruts caches the list of struts in its layout modifier instance and updates it via messages

Yes, but making cache layout-local is required. I described the reason in the comments to 28e9f8b.
UPD: Making strutSet does not solve the issue because layout requires sending messages to update itself(or do you know better way to update layout?). And anyway messages are sent to wrong layout when using X.L.PerWorkspace

@davama
Copy link
Contributor Author

davama commented Sep 1, 2016

@liskin actually all i did was run dzen2 with the -dock option and that seemed to work. (xmonad restart, logout, reboot) (tested with git and haskel xmonad)

Interestingly i noticed that with the -dock option i had to change my layout reset binding:

-- , ((mod .|. shiftMask, xK_space), setLayout $ XMonad.layoutHook conf)
, ((mod .|. shiftMask, xK_space), (setLayout $ XMonad.layoutHook conf) >> docksStartupHook) 

Not sure why but some googling brought me here and here

Not sure if it is a dzen issue or xmonad issue at this point.
You can close this if you like.

Edit:
I've updated cloned git xmonad and now docksStartupHook fails. Reverted to previous keybinding:

, ((mod .|. shiftMask, xK_space), setLayout $ XMonad.layoutHook conf)

Things work as expected. Window clients respect dzen dock when using X.L.PerWorkspace

@geekosaur
Copy link
Contributor

On Thu, Sep 1, 2016 at 2:13 PM, Dave [email protected] wrote:

Interestingly i noticed that with the -dock option i had to change my
layout reset binding

Because the struts are (incorrectly) being cached in the data associated
with the constructor for avoidStruts, which means it gets set to default
(empty) when the layout is reset --- that set-to-default being one of the
reasons you'd want to reset a layout, since the expectation is that such
data has to do with things like the size of the master area or resize
increment, not hidden data. It should be in ExtensibleState instead.

brandon s allbery kf8nh sine nomine associates
[email protected] [email protected]
unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

@liskin
Copy link
Member

liskin commented Sep 1, 2016

Just a note for @f1u77y, had the strutMap (

strutMap :: M.Map Window (Maybe (Either [CLong] [CLong]))
) been a Maybe like avoidStrutsRectCache, the docksStartupHook would probably be unnecessary. The bug with multiple instances of AvoidStruts would still be there, but at least resetting a layout would invalidate the cache instead of caching an incorrect (empty) value.

@geekosaur
Copy link
Contributor

geekosaur commented Sep 1, 2016 via email

@liskin
Copy link
Member

liskin commented Sep 1, 2016

The "two different kinds of relayout" thing will be solved by having the cache in extensible state instead of the layout instance.

@f1u77y
Copy link
Contributor

f1u77y commented Sep 1, 2016

@geekosaur as far as i can understand, you suggest to recalc cache in modifyLayoutWithUpdate for doing it without messages. But won't it cause bad handling of new-coming docks?

@davama
Copy link
Contributor Author

davama commented Sep 13, 2016

Update:
I noticed docksStartupHook fails when in my xmonad.hs file. Searched around and found #80 . They did mention "this" PR, whether it could be closed, but @f1u77y said something about messages not being sent to the layout.

As far as my initial issue, docks not being respected, that is fixed. So from my perspective this can close, but im not a developer.

Thank you all for the support.

-Dave

@liskin
Copy link
Member

liskin commented Sep 13, 2016

@davama What do you mean by "docksStartupHook fails"? It shouldn't. :-)

@davama
Copy link
Contributor Author

davama commented Sep 13, 2016

@liskin
I get this error:

xmonad.hs:480:76: error:
    Variable not in scope: docksStartupHook :: X ()

That line looks like this:
, ((mod .|. shiftMask, xK_space), (setLayout $ XMonad.layoutHook conf) >> docksStartupHook)

So i just went back to:
, ((mod .|. shiftMask, xK_space), setLayout $ XMonad.layoutHook conf)

and things work fine again.
The reason i had to change it using docksStartupHook was because the docks would get overlapped by the windows. As of right now, latest git pull, docksStartupHook fails for me but old method of resseting the layout works again. ( thought because of #80 changes )

Hope it's clear.

@liskin
Copy link
Member

liskin commented Sep 13, 2016

Oh, you probably just didn't import it. And I guess you didn't use this: e38fb3b#diff-79e50f9693ab1653ba2edaa25759c9f8R88 either, did you?

@davama
Copy link
Contributor Author

davama commented Sep 13, 2016

@liskin you were right, i didnt. i thought it was applied to the master already. should have double checked.
Well, i imported all the changes, rebuild xmonad, reloaded xmonad, but failed. Still get Variable not in scope: docksStartupHook :: X ()

Can't seem to figure out where to add this docks def in my main. I've tried different places but cant figure it out.

Snippet of my config:

defaults hostname = ewmh defaultConfig {
 terminal = myTerminal
 , focusFollowsMouse = myFocusFollowsMouse
 , clickJustFocuses = myClickJustFocuses
......
}

main = do
 hostname <- fmap nodeName getSystemID
 xmproc <- spawnPipe myDzenStatus
 xmonad $ withUrgencyHookC LibNotifyUrgencyHook myUrgencyConfig
        $ withNavigation2DConfig defaultNavigation2DConfig
        $ (defaults hostname) {
          manageHook = composeAll [
                      .....
          ]
          , startupHook = composeAll [
                     myStartupHook
                     , setWMName "Xmonad"
                     , myCaseHook hostname
                     , myDzenStartup
          ]
          , logHook = composeAll [
                .......
          ]
          >> updatePointer (0.5, 0.5) (0, 0) -- moves pointer to center of focused window
 }

Thanks!

@liskin
Copy link
Member

liskin commented Sep 13, 2016

Perhaps you're missing an import?
Basically you should do what I did to XMonad/Config/Droundy.hs in e38fb3b

@davama
Copy link
Contributor Author

davama commented Sep 13, 2016

The import is there import XMonad.Hooks.ManageDocks

i did try this: (perhaps doing it wrong)
defaults hostname = docks $ ewmh defaultConfig { ....

but got a crazy error here xmonad-contrib-#76-error

My layout looks like so:

gapper s = (gaps [(U, s), (R, s), (L, s), (D, s)]) . (spacing s)
grid orientation s = gapper s $ G.SplitGrid orientation 1 1 (1/2) (16/9) (5/100)
myLayoutHook = onWorkspace (myWorkspaces !! 8) (avoidStruts (imLayout) ||| avoidStruts (standardLayouts)) -- for pidgin
             $ avoidStruts (standardLayouts) ||| fullscreen
  where
    standardLayouts = named "+" (spacing myBorderSpace $ G.Grid (16/10)) -- grid
                    ||| named "TT" (grid G.T myBorderSpace) -- grid ; Tall ; spacing
                    ||| named "[]-" (grid G.L myBorderSpace) -- grid ; left ; spacing
    fullscreen = named "[_]" (noBorders (fullscreenFull Full)) -- full
    imLayout = gridIM (1/5) (Title "Buddy List") -- actual name "IM Grid"

I double checked the xmonad src code i built from, and i can confirm that the #80 changes were made.
I deleleted my ~/.cabal ~/.ghc folders and rebuild xmonad again, just in case. (only way i know how to do it)

@liskin
Copy link
Member

liskin commented Sep 13, 2016

It really looks like you're compiling your config against an xmonad version that does not have the #80 changes. If you really have import XMonad.Hooks.ManageDocks (without additional parentheses after it) there, that is really the only possible explanation.

@davama
Copy link
Contributor Author

davama commented Sep 13, 2016

(without additional parentheses after it) there, that is really the only possible explanation.

That's what i was thinking too. That's why i checked again

Well i did the rebuild once more. (perhaps my rebuild process is wrong)

My order of operations:
rm .cabal .ghc -rf
cd xmonad-test/

ls
xmonad  xmonad-contrib  xmonad-extras

grep -i "docks " xmonad-contrib/XMonad/*/*

xmonad-contrib/XMonad/Actions/FloatSnap.hs:import XMonad.Hooks.ManageDocks (calcGap)
xmonad-contrib/XMonad/Config/Bluetile.hs:    docks $
xmonad-contrib/XMonad/Config/Desktop.hs:desktopConfig = docks $ ewmh def
xmonad-contrib/XMonad/Config/Dmwit.hs:dmwitConfig nScreens = docks $ def {
xmonad-contrib/XMonad/Config/Droundy.hs:import XMonad.Hooks.ManageDocks ( avoidStruts, docks )
xmonad-contrib/XMonad/Config/Droundy.hs:config = docks $ ewmh def
xmonad-contrib/XMonad/Config/Sjanssen.hs:    docks $ ewmh $ def
xmonad-contrib/XMonad/Hooks/DynamicLog.hs:-- The binding uses the XMonad.Hooks.ManageDocks module to automatically
xmonad-contrib/XMonad/Hooks/DynamicLog.hs:    return $ docks $ conf
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:module XMonad.Hooks.ManageDocks (
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:-- > main = xmonad $ docks def
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:-- If you want certain docks to be avoided but others to be covered by
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:-- docks should be avoided, using 'avoidStrutsOn'.  For example:
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:-- | Add docks functionality to the given config.  See above for an example.
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:docks :: XConfig a -> XConfig a
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:docks c = c { startupHook     = docksStartupHook <+> startupHook c
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:            , manageHook      = manageDocks <+> manageHook c }
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:data UpdateDocks = UpdateDocks deriving Typeable
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:refreshDocks :: X ()
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:refreshDocks = sendMessage UpdateDocks
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:manageDocks :: ManageHook
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:manageDocks = checkDock --> (doIgnore <+> setDocksMask)
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:    docks <- filterM (runQuery checkDock) wins
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:    forM_ docks $ \win -> do
xmonad-contrib/XMonad/Hooks/ManageDocks.hs:        | Just UpdateDocks <- fromMessage m = Just as
xmonad-contrib/XMonad/Hooks/PositionStoreHooks.hs:                sr' <- fmap ($ sr) (calcGap $ S.fromList [minBound .. maxBound]) -- take docks into account, accepting
xmonad-contrib/XMonad/Layout/Gaps.hs:-- ManageDocks does not work; for example, to work with a dock-type

/usr/bin/cabal update
/usr/bin/cabal install cabal-install
for i in $(ls); do cabal install $i; done note cabal alias points to ~/.cabal/bin/cabal
cd ~/.xmonad
rm xmonad.hi xmonad.o xmonad.errors xmonad-x86_64-linux
xmonad --recompile note: xmonad alias points to ~/.cabal/bin/xmonad

xmonad fails.

Thanks again

@liskin
Copy link
Member

liskin commented Sep 13, 2016

I'm out of ideas. :-(

@f1u77y
Copy link
Contributor

f1u77y commented Sep 13, 2016

@davama cabal install $arg does install from the hackage, try (cd $i && cabal install)

@davama
Copy link
Contributor Author

davama commented Sep 13, 2016

cabal install $arg does install from the hackage, try (cd $i && cabal install)

@f1u77y you were right, thank you
Im so stupid. Sorry about that @liskin

OK, xmonad --recompile works.

currently testing...

@liskin
Copy link
Member

liskin commented Sep 13, 2016

Yeah, good catch. :-)

@davama
Copy link
Contributor Author

davama commented Sep 13, 2016

Playing around with different scenarios:
xmonad login
xmonad restart
toggleStruts
setLayout
creating dzen -dock bar on the fly

and at the same time figuring out what i dont need in my config:
Things i dont need: (at least i think)

docksEventHook e
manageDocks
, ((mod .|. shiftMask, xK_space), (setLayout $ XMonad.layoutHook conf) >> docksStartupHook)

Replaced or added with:

defaults hostname = docks $ ewmh defaultConfig {
, ((mod .|. shiftMask, xK_space), setLayout $ XMonad.layoutHook conf)

In every scenario i can think of, the docks are respected every time. Dont know who manages the documentation but that might need an update if i did this correctly. Doc

Anything else to try?
Thanks

@f1u77y
Copy link
Contributor

f1u77y commented Sep 13, 2016

@davama

but that might need an update

ofc it does, is's for 0.11 version
try building local docs(cabal install --haddock-all will place them somewhere in ~/.cabal/doc)
there was a thread about docs somewhere(github or mailing list) but as I can see devs don't have too much free time

@davama
Copy link
Contributor Author

davama commented Sep 13, 2016

is's for 0.11 version

I see.
Thanks for the tip @f1u77y .
I also remember seeing a thread about the docs somewhere.

I'll keep an eye on #80 as to when they'll be merged with the master.

I think this can be closed.

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants