X.H.DynamicLog: Move UTF8 encoding from dynamicLogString to xmonadPropLog'

For many (10+) years, we had a cascade of ugly workarounds:

 * X.U.Run.spawnPipe returned a binary handle, so the String passed to
   it must have been encoded by C.B.UTF8.String.encodeString

 * X.H.DynamicLog.dynamicLogString therefore returned such an encoded
   String, so one could use it directly in a hPutStrLn, but literal
   Strings wouldn't work

 * xmonadPropLog' also expected an encoded String to make it easier to
   use together with dynamicLogString, again breaking usage with String
   literals and other normal unicode Strings

Then in 1d0eaddc25 Sibi fixed spawnPipe to return a handle usable with
normal Strings, which then obviously broke the entire cascade. But,
instead of using the opportunity to remove all the ugly workarounds, he
decided to add some more on top, so now spawnPipe with dynamicLogString
outputs doubly encoded UTF-8 and xmobar has a hack to strip this double
encoding (https://github.com/jaor/xmobar/pull/482), which only works
when XFT is in use and breaks on some long unicode codepoints. :-(

There is a better way: make everything just use normal Strings and only
encode when it goes out the wire. This means dynamicLogString can be
freely mixed with String literals, manual uses of xmonadPropLog' don't
need encodeString, and everything just works nicely.

This obviously breaks configs that used some of these pieces in
isolation (like mine), but that's a small price to pay. After all, right
now all users of spawnPipe/dynamicLogString are getting doubly encoded
UTF-8 which might or might not work in xmobar and probably breaks
horribly everywhere else, so this fix should be a clear improvement. :-)

Fixes: 1d0eaddc25 ("Make spawnPipe to use system's locale encoding")
Fixes: https://github.com/xmonad/xmonad-contrib/issues/377
Fixes: https://github.com/jaor/xmobar/issues/476
Related: https://github.com/xmonad/xmonad-contrib/pull/334
Related: https://github.com/jaor/xmobar/pull/482

CHANGES.md

@@ -50,6 +50,26 @@
     - The type of `getWorkspaceNames` was changed to fit into the new `ppRename`
       field of `PP`.
 
+  * `XMonad.Hooks.DynamicLog` and `XMonad.Util.Run`
+
+    - `spawnPipe` no longer uses binary mode handles but defaults to the
+      current locale encoding instead.
+
+      `dynamicLogString`, the output of which usually goes directly into such
+      a handle, no longer encodes its output in UTF-8, but returns a normal
+      `String` of Unicode codepoints instead.
+
+      When these two are used together, everything should continue to work as
+      it always has, but in isolation behaviour might change.
+
+      (To get the old `spawnPipe` behaviour, `spawnPipeWithNoEncoding` can now
+      be used, and `spawnPipeWithUtf8Encoding` was added as well to force
+      UTF-8 regardless of locale. These shouldn't normally be necessary, though.)
+
+    - `xmonadPropLog` and `xmonadPropLog'` now encode the String in UTF-8.
+      Again, no change when used together with `dynamicLogString`, but other
+      uses of these in user configs might need to be adapted.
+
 ### New Modules
 
   * `XMonad.Util.Hacks`
@@ -271,16 +291,6 @@
 
      - Exported the `scratchpadWorkspaceTag`.
 
-  * `XMonad.Util.Run`
-
-     - Added two new functions to the module:
-       `spawnPipeWithLocaleEncoding` and
-       `spawnPipeWithUtf8Encoding`. `spawnPipe` is now alias for
-       `spawnPipeWithLocaleEncoding`.
-
-     - Added the function `spawnPipeWithNoEncoding` for cases where a
-       binary handle is required.
-
   * `XMonad.Prompt.Window`
 
     - Added `allApplications` function which maps application executable

XMonad/Hooks/DynamicLog.hs

@@ -80,12 +80,12 @@ module XMonad.Hooks.DynamicLog (
 
 -- Useful imports
 
-import Codec.Binary.UTF8.String (encodeString)
+import qualified Codec.Binary.UTF8.String as UTF8 (encode)
 import Control.Applicative (liftA2)
 import Control.Exception (SomeException, try)
 import Control.Monad (msum, void)
 import Data.Bool (bool)
-import Data.Char (isSpace, ord)
+import Data.Char (isSpace)
 import Data.List (intercalate, isPrefixOf, sortOn, stripPrefix)
 import Data.Maybe (catMaybes, fromMaybe, isJust, mapMaybe)
 import System.Posix.Signals (sigTERM, signalProcessGroup)
@@ -492,8 +492,7 @@ makeSB sb k conf = do
   makeStatusBar' sb' k conf
 
 -- | Write a string to a property on the root window.  This property is of type
--- @UTF8_STRING@. The string must have been processed by 'encodeString'
--- ('dynamicLogString' does this).
+-- @UTF8_STRING@.
 xmonadPropLog' :: String  -- ^ Property name
                -> String  -- ^ Message to be written to the property
                -> X ()
@@ -505,7 +504,7 @@ xmonadPropLog' prop msg = do
     io $ changeProperty8 d r xlog ustring propModeReplace (encodeCChar msg)
  where
     encodeCChar :: String -> [CChar]
-    encodeCChar = map (fromIntegral . ord)
+    encodeCChar = map fromIntegral . UTF8.encode
 
 -- | Write a string to the @_XMONAD_LOG@ property on the root window.
 xmonadPropLog :: String -> X ()
@@ -570,7 +569,7 @@ dynamicLogString pp = do
     -- run extra loggers, ignoring any that generate errors.
     extras <- mapM (`catchX` return Nothing) $ ppExtras pp
 
-    return $ encodeString . sepBy (ppSep pp) . ppOrder pp $
+    return $ sepBy (ppSep pp) . ppOrder pp $
                         [ ws
                         , ppLayout pp ld
                         , ppNames