Add configurable workspace numbering (#2)

fix: Expand cl-lib macros at compile time

README.org

@@ -1,5 +1,7 @@
 #+TITLE: exwm-modeline
 
+[[https://melpa.org/#/exwm-modeline][file:https://melpa.org/packages/exwm-modeline-badge.svg]]
+
 A modeline segment to display exwm workspaces.
 
 Here's how it looks near the list of [[https://github.com/nex3/perspective-el][perspectives]] (the segment of the current package is to the left):
@@ -13,15 +15,11 @@ Features:
 - Numbers are clickable.
 
 * Installation
-As the package isn't yet available anywhere but in this repository, you can clone the repository, add it to the =load-path=, and =require= the package:
-#+begin_src emacs-lisp
-(require 'exwm-modeline)
-#+end_src
+The package is available on MELPA. Install it however you usually install packages, I use [[https://github.com/jwiegley/use-package][use-package]] and [[https://github.com/raxod502/straight.el][straight.el]]:
 
-My preferred way is to use =use-package= with =straight=:
 #+begin_src emacs-lisp
 (use-package exwm-modeline
-  :straight (:host github :repo "SqrtMinusOne/exwm-modeline")
+  :straight t
   :after (exwm))
 #+end_src
 
@@ -35,6 +33,8 @@ Set =exwm-modeline-randr= to nil to turn off filtering of workspaces by monitor.
 
 Set =exwm-modeline-short= to =t= display only the current workspace in the modeline.
 
+Set =exwm-modeline-display-urgent= to nil to turn off displaying whether a workspace has an urgent window. This will significantly decrease the number of modeline updates, which may help with performance issues.
+
 * Credits
 [[https://github.com/nex3/perspective-el][perspective.el]] by [[https://github.com/nex3][@nex3]] was extremely instructive on how to make a modeline segment individual to a particular frame and avoid recalculating it too often.
 

exwm-modeline.el

@@ -1,14 +1,15 @@
 ;;; exwm-modeline.el ---  A modeline segment for EXWM workspaces -*- lexical-binding: t -*-
 
-;; Copyright (C) 2021 Korytov Pavel
+;; Copyright (C) 2021-2023 Korytov Pavel
 ;; Copyright (C) 2021 Ellis Kenyő
 ;; Copyright (C) 2008-2020 Natalie Weizenbaum <nex342@gmail.com>
 
 ;; Author: Korytov Pavel <thexcloud@gmail.com>
 ;; Maintainer: Korytov Pavel <thexcloud@gmail.com>
-;; Version: 0.1.1
+;; Version: 0.1.3
 ;; Package-Requires: ((emacs "27.1") (exwm "0.26"))
-;; Homepage: https://github.com/SqrtMinusOne/pomm.el
+;; Homepage: https://github.com/SqrtMinusOne/exwm-modeline
+;; Published-At: 2021-12-22
 
 ;; This file is NOT part of GNU Emacs.
 
@@ -37,6 +38,10 @@
 ;; Take a look at `exwm-modeline-mode' for more info.
 
 ;;; Code:
+(eval-when-compile
+  (require 'cl-lib))
+(require 'exwm)
+(require 'exwm-randr)
 (require 'exwm-workspace)
 (require 'exwm-manage)
 
@@ -47,7 +52,7 @@
 (defcustom exwm-modeline-dividers '("[" "]" "|")
   "Plist of strings used to create the string shown in the modeline.
 
-First string is the start of the modestring, second is the
+The first string is the start of the modestring, second is the
 closing of the modestring, and the last is the divider between
 workspaces."
   :group 'exwm-modeline
@@ -56,20 +61,59 @@ workspaces."
                (string :tag "Divider")))
 
 (defcustom exwm-modeline-short nil
-  "When t, display only the current workspace in the modeline."
+  "When set, display only the current workspace in the modeline."
   :group 'exwm-modeline
-  :type 'boolean)
+  :type 'boolean
+  :set (lambda (sym value)
+         (set-default sym value)
+         (when (bound-and-true-p exwm-modeline-mode)
+           (exwm-modeline-update))))
 
 (defcustom exwm-modeline-randr t
-  "When t, show only workspaces on the current monitor."
+  "When set, only show workspaces on the current monitor."
   :group 'exwm-modeline
-  :type 'boolean)
+  :type 'boolean
+  :set (lambda (sym value)
+         (set-default sym value)
+         (when (bound-and-true-p exwm-modeline-mode)
+           (exwm-modeline-update))))
+
+(defcustom exwm-modeline-display-urgent t
+  "When set, display the urgent status in the modeline.
+
+With that set, the modeline will be updated on every workspace
+switch, so the number of updates is increased significantly."
+  :group 'exwm-modeline
+  :type 'boolean
+  :set (lambda (sym value)
+         (set-default sym value)
+         (when (bound-and-true-p exwm-modeline-mode)
+           (exwm-modeline-update)
+           (if value
+               (progn
+                 (advice-add #'exwm--update-hints :after #'exwm-modeline--urgency-advice)
+                 (add-hook 'exwm-workspace-switch-hook #'exwm-modeline--urgency-advice))
+             (advice-remove #'exwm--update-hints #'exwm-modeline--urgency-advice)
+             (remove-hook 'exwm-workspace-switch-hook #'exwm-modeline--urgency-advice)))))
+
+(defcustom exwm-modeline-workspace-index-map 'exwm-workspace-index-map
+  "Function for mapping a workspace index to a string for display.
+
+If the value if 'exwm-workspace-index-map, dereference the
+`exwm-workspace-index-map'variable.  Otherwise, use a function."
+  :group 'exwm-workspace
+  :type '(choice
+          (symbol exwm-workspace-index-map :tag "Use EXWM default")
+          (function))
+  :set (lambda (sym value)
+         (set-default sym value)
+         (when (bound-and-true-p exwm-modeline-mode)
+           (exwm-modeline-update))))
 
 (defface exwm-modeline-current-workspace
-  ;; I'd rather :inherit warning there, but that could lead to
-  ;; unexpected effects.
+  ;; I'd rather :inherit and override warning there, but well
   `((t :foreground ,(face-foreground 'warning) :weight bold))
-  "Face for the current workspace. "
+  "Face for the current workspace."
   :group 'exwm-modeline)
 
 (defface exwm-modeline-populated-workspace
@@ -78,18 +122,21 @@ workspaces."
   :group 'exwm-modeline)
 
 (defface exwm-modeline-empty-workspace
-  `((t (:foreground ,(face-foreground 'mode-line))))
+  `((t (:inherit mode-line)))
   "Face for any workspace without an X window."
   :group 'exwm-modeline)
 
 (defface exwm-modeline-urgent-workspace
-  '((t (:inherit custom-invalid)))
+  '((t (:inherit error)))
   "Face for any workspace that is tagged as urgent by X."
   :group 'exwm-modeline)
 
 (defun exwm-modeline--urgent-p (frame)
-  "Determine if FRAME is tagged as urgent."
-  (frame-parameter frame 'exwm-urgency))
+  "Determine if FRAME is tagged as urgent.
+
+Always return nil if `exwm-modeline-display-urgent' is not set."
+  (when exwm-modeline-display-urgent
+    (frame-parameter frame 'exwm-urgency)))
 
 (defun exwm-modeline--populated-p (frame)
   "Determine if FRAME has any X windows."
@@ -97,6 +144,14 @@ workspaces."
            if (eq frame (buffer-local-value 'exwm--frame (cdr item)))
            return t))
 
+(defun exwm-modeline--workspace-index-map (i)
+  "Map the workspace index I to a string for display.
+
+See `exwm-modeline-workspace-index-map' for behaviour."
+  (if (eq exwm-modeline-workspace-index-map 'exwm-workspace-index-map)
+      (funcall exwm-workspace-index-map i)
+    (funcall exwm-modeline-workspace-index-map i)))
+
 (defun exwm-modeline--click (event)
   "Process a click EVENT on the modeline segment."
   (interactive "e")
@@ -104,7 +159,7 @@ workspaces."
       (target
        (cl-loop with name = (format "%s" (car (posn-string (event-start event))))
                 for i from 0 to (1- (length exwm-workspace--list))
-                if (string-equal (funcall exwm-workspace-index-map i) name)
+                if (string-equal (exwm-modeline--workspace-index-map i) name)
                 return i))
     (exwm-workspace-switch target)))
 
@@ -120,7 +175,7 @@ workspaces."
 WORKSPACE-LIST is the list of frames to display."
   (cl-loop for frame in workspace-list
            for i from 0 to (length workspace-list)
-           for workspace-name = (funcall exwm-workspace-index-map
+           for workspace-name = (exwm-modeline--workspace-index-map
                                  (exwm-workspace--position frame))
            with current-frame = (selected-frame)
            if (= i 0) collect (nth 0 exwm-modeline-dividers)
@@ -144,13 +199,13 @@ WORKSPACE-LIST is the list of frames to display."
 
 (defun exwm-modeline--randr-workspaces ()
   "Get workspaces on the same monitor as current frame."
-  (if-let ((monitor (plist-get exwm-randr-workspace-output-plist
+  (if-let ((monitor (plist-get exwm-randr-workspace-monitor-plist
                                (cl-position (selected-frame)
                                             exwm-workspace--list)))
            ;; This list of frame actually can be wrongly ordered,
            ;; hence the second loop.  The number of values is quite
            ;; small, so it's not like o(n^2) can cause any issues.
-           (frames (cl-loop for (key value) on exwm-randr-workspace-output-plist
+           (frames (cl-loop for (key value) on exwm-randr-workspace-monitor-plist
                             by 'cddr
                             if (string-equal value monitor)
                             collect (nth key exwm-workspace--list))))
@@ -158,7 +213,7 @@ WORKSPACE-LIST is the list of frames to display."
                if (member frame frames)
                collect frame)
     (cl-loop with indices = (cl-loop
-                             for (key value) on exwm-randr-workspace-output-plist
+                             for (key _) on exwm-randr-workspace-monitor-plist
                              by 'cddr collect key)
              for i from 0 to (1- (length exwm-workspace--list))
              for frame in exwm-workspace--list
@@ -186,14 +241,41 @@ WORKSPACE-LIST is the list of frames to display."
   (frame-parameter nil 'exwm-modeline--string))
 
 (defun exwm-modeline--unmanage-advice (&rest _)
-  "An advice to update the modeline.
+  "Update the modeline after unmanaging a window.
 
-This one is meant to be attached :after
-`exwm-manage--unmanage-window', because that's when a workspace
-can lose all its X windows and thus may become \"unpopulated\",
-i.e. the face in the segment has to change."
+This function is meant to be advised :after
+`exwm-manage--unmanage-window', because that's when a workspace can
+lose all its X windows and thus may become \"unpopulated\",i.e. the
+face in the segment has to change."
   (exwm-modeline-update))
 
+(defun exwm-modeline--urgency-advice (&rest _)
+  "Update the modeline after a change in the urgency status.
+
+This function is meant to be advised :after `exwm--update-hints' and
+be in the hook `exwm-workspace-switch-hook'.
+
+The modeline is updated if `exwm-workspace--switch-history-outdated'
+is set to t, because EXWM sets that variable whenever a window updates
+it urgency status.  To avoid running the function too often,
+`exwm-workspace--update-switch-history' is also called, which resets
+the variable.
+
+However, one issue with the mentioned variable is that it is also
+set whenever the workspace is switched, so using that advice also
+increases the number of required updates.  Optimizing that would
+require more substantial modifications to EXWM code, so in this
+package applying that advice is made optional with the
+`exwm-modeline-display-urgent' variable.
+
+Because the first function is very much critical for the normal
+functioning of EXWM, the entire thing is wrapped in
+`with-demoted-errors'."
+  (with-demoted-errors "Error in exwm-modeline--urgency-advice: %S"
+    (when exwm-workspace--switch-history-outdated
+      (exwm-modeline-update)
+      (exwm-workspace--update-switch-history))))
+
 ;;;###autoload
 (define-minor-mode exwm-modeline-mode
   "A mode for displaying EXWM workspaces in the modeline.
@@ -206,10 +288,16 @@ monitor.  To display only the current workspace, enable
 `exwm-modeline-short', and to disable the filtering by the
 monitor, disable `exwm-modeline-randr'.
 
-Also take a look at the `exwm-modeline' group for faces
+If `exwm-modeline-display-urgent' is set, the mode also displays
+if the workspace has a window market as urgent.  However, this
+option forces the modeline to update after every workspace
+switch, so it may be wise to disable that in case of performance
+issues.
+
+Also, take a look at the `exwm-modeline' group for faces
 customization.
 
-This implementation indents to reduce the count of times of
+This implementation intends to reduce the count of times of
 evaluating the modestring; the rendered modestring is saved as a
 frame parameter, and `exwm-modeline-segment' just returns it.
 
@@ -226,14 +314,19 @@ cases when the workspace list changes."
         (add-hook 'exwm-randr-refresh-hook #'exwm-modeline-update)
         (add-hook 'exwm-manage-finish-hook #'exwm-modeline-update)
         (advice-add #'exwm-manage--unmanage-window
-                    :after #'exwm-modeline--unmanage-advice))
+                    :after #'exwm-modeline--unmanage-advice)
+        (when exwm-modeline-display-urgent
+          (advice-add #'exwm--update-hints :after #'exwm-modeline--urgency-advice)
+          (add-hook 'exwm-workspace-switch-hook #'exwm-modeline--urgency-advice)))
     (setq global-mode-string (delete '(:eval (exwm-modeline-segment))
                                      global-mode-string))
     (remove-hook 'exwm-workspace-list-change-hook #'exwm-modeline-update)
     (remove-hook 'exwm-randr-refresh-hook #'exwm-modeline-update)
     (remove-hook 'exwm-manage-finish-hook #'exwm-modeline-update)
     (advice-remove #'exwm-manage--unmanage-window
-                   #'exwm-modeline--unmanage-advice)))
+                   #'exwm-modeline--unmanage-advice)
+    (advice-remove #'exwm--update-hints #'exwm-modeline--urgency-advice)
+    (remove-hook 'exwm-workspace-switch-hook #'exwm-modeline--urgency-advice)))
 
 (provide 'exwm-modeline)
 ;;; exwm-modeline.el ends here