`XMonad.Hooks.Modal.logMode` used in `XMonad.Hooks.StatusBar.statusBarProp` issue

[sagittarius-a]

Problem Description

When I use the XMonad.Hooks.Modal.logMode in the ppExtras field of xmobar pretty-printer, it works flawlessly.

However, if I use ppOrder:

• the ordering is properly applied
• it works as soon as I enter the new mode, the mode string is printed to xmobar
• I can perform actions in the current mode

But:

• But I cannot exit the mode anymore
• The mode string is still in xmobar (though I can't tell if it is a cache issue in xmobar)
• No other Xmonad keybindings works

The only way to solve the issue is to restart Xmonad. It can be very tedious if there is no way to do it without using any Xmonad keybinding.

Steps to Reproduce

• [Optional] Open a terminal (to be able to restart Xmonad easily)
• Press M-g to enter the example mode
• Press Escape to exit the example mode

No Xmonad keybinding should be working at this point.

Configuration File

import XMonad
import XMonad.Hooks.StatusBar
import XMonad.Hooks.StatusBar.PP
import XMonad.Util.EZConfig
import XMonad.Hooks.Modal

main :: IO ()
main = xmonad
     . modal [noModMode, sayHelloMode]
     . withEasySB (statusBarProp "xmobar" (pure myXmobarPP)) defToggleStrutsKey
     $ def
  `additionalKeysP`
    [ ("M-g"  , setMode "Hello")   ]

myXmobarPP :: PP
myXmobarPP = def
    { ppOrder           = \[ws, l, title, mode] -> [ws, l, mode, title]
    , ppExtras          = [logMode]
    }

    -- { ppExtras          = [logMode] }

sayHelloMode :: Mode
sayHelloMode = mode "Hello" $ mkKeysEz
  [ ("g", xmessage "Hello, World!") ]

Checklist

• [X] I've read CONTRIBUTING.md

• I tested my configuration

  • [X] With xmonad commit 391c0fc
  • [X] With xmonad-contrib commit 29f0e032

[LSLeary]

A sequence of events:

1. You run exitMode.
2. There's no longer an active mode, so logMode produces Nothing.
3. Hence the list that ppOrder takes shrinks to length 3.
4. ppOrder doesn't cover the case; pattern match failure kills dynamicLogString, dynamicLogWithPP, and hence the logHook.
5. Flowing on, the logHook kills refreshMode, which kills setMode and exitMode.

In short exitMode becomes a self-destruct button. XMonad recovers from bad keybindings like this by pretending they never happened.

The solution is to write ppOrder as a total function:

    { ppOrder           = \case { [ws, l, title, mode] -> [ws, l, mode, title]; xs -> xs }

[geekosaur]

We should probably wrap ppOrder in userCodeDef so it doesn't cause as much carnage on failure.

[geekosaur]

https://github.com/xmonad/xmonad-contrib/blob/master/XMonad/Hooks/StatusBar/PP.hs#L202

[LSLeary]

The actual error is in pure code, so it's a pain to catch without letting it kill all the code that eventually evaluates it. Though we already depend on deepseq, so I guess something like the below (untested) should work.

  let pps = [ws, ppLayout pp ld, ppTitle pp $ ppTitleSanitize pp wt]
         ++ catMaybes extras
  sepBy (ppSep pp) <$> userCodeDef pps (io . evaluate . force $ ppOrder pp pps)

[LSLeary]

Frankly I'd rather just change whatever documentation suggests writing a partial function there and call it user error from then on, but that won't fix all the broken configs in the wild.

[geekosaur]

I'd agree in the ideal case, but when it allows something like this bug report it seems like something we should try to deal with. And it's going to get forced shortly anyway, so I see no problem with deepseq there.

[geekosaur]

Actually, I now wonder if the userCode belongs on dynamicLogPP or StatusBar equivalent, since nothing but convention stops this from happening in any other of the PP functions. That leaves out dynamicLogString, but I think anyone who knows enough to use that directly knows to avoid partial functions or pay the price.

[LSLeary]

I grepped for dynamicLogString in contrib and it seemed to be used in several places, but I don't know the details of these modules, so I'll leave it up to people who do.

[sagittarius-a]

A sequence of events:

1. You run `exitMode`.

2. There's no longer an active mode, so `logMode` produces `Nothing`.

3. Hence the list that `ppOrder` takes shrinks to length 3.

4. `ppOrder` doesn't cover the case; pattern match failure kills `dynamicLogString`, `dynamicLogWithPP`, and hence the `logHook`.

5. Flowing on, the `logHook` kills `refreshMode`, which kills `setMode` and `exitMode`.

In short exitMode becomes a self-destruct button. XMonad recovers from bad keybindings like this by pretending they never happened.

The solution is to write ppOrder as a total function:

    { ppOrder           = \case { [ws, l, title, mode] -> [ws, l, mode, title]; xs -> xs }

Well that's a fast answer with a solution to my problem.

Thank you very much :)

[geekosaur]

I grepped for dynamicLogString in contrib and it seemed to be used in several places, but I don't know the details of these modules, so I'll leave it up to people who do.

Looks to me like what I'd expect. It should be easy enough, since it's in X String, to use userCodeDef (pure "_|_") in front or `catchX` pure "_|_" at the end. Except I guess it needs to force the string to make sure any exception occurs.

[geekosaur]

PR #802 opened.