Release: v0.5

README.org

@@ -52,17 +52,19 @@ This is the recommended configuration, in terms of a ~use-package~ form to be pl
     :init
     (activities-mode)
     (activities-tabs-mode)
+    ;; Prevent `edebug' default bindings from interfering.
+    (setq edebug-inhibit-emacs-lisp-mode-bindings t)
 
     :bind
-    (("C-x C-a n" . activities-new)
-     ("C-x C-a g" . activities-revert)
-     ("C-x C-a s" . activities-suspend)
-     ("C-x C-a C-k" . activities-kill)    ; Alias for `-suspend'
-     ("C-x C-a a" . activities-resume)
-     ;; For convenience, we also bind `activities-resume' to "C-x C-a
-     ;; C-a", so the user need not lift the Control key.
+    (("C-x C-a C-n" . activities-new)
+     ;; As resuming is expected to be one of the most commonly used
+     ;; commands, this binding is one of the easiest to press.
      ("C-x C-a C-a" . activities-resume)
+     ("C-x C-a C-s" . activities-suspend)
+     ("C-x C-a C-k" . activities-kill)
+     ;; This binding mirrors, e.g. "C-x t RET".
      ("C-x C-a RET" . activities-switch)
+     ("C-x C-a g" . activities-revert)
      ("C-x C-a l" . activities-list)))
 #+END_SRC
 
@@ -97,12 +99,19 @@ An example of a workflow using activities:
 8. Call ~activities-new~ with a universal prefix argument (~C-u C-x C-a n~) to redefine an activity's default state.
 9. Suspend the activity with ~activities-suspend~ (~C-x C-a s~) (which saves its last state and closes its frame/tab).
 
+** Bindings
+
+Key bindings are, as always, ultimately up to the user.  However, in [[Configuration][Configuration]], we suggest a set of bindings with a simple philosophy behind them:
+
++ A binding ending in a ~C~-prefixed key is expected to result in the set of active activities being changed (e.g. defining a new activity, activating one, or deactivating one).
++ A binding not ending in a ~C~-prefixed key is expected to merely change an active one (e.g. reverting it) or do something else (like listing activities.)
+
 ** Commands
 
 + ~activities-list~ (~C-x C-a l~) :: List activities in a ~vtable~ buffer in which they can be managed with various commands.
-+ ~activities-new~ (~C-x C-a n~) :: Define a new activity whose default state is the current frame's or tab's window configuration.  With prefix argument, overwrite an existing activity (thereby updating its default state to the current state).
-+ ~activities-suspend~ (~C-x C-a s~) :: Save an activity's state and close its frame or tab.
-+ ~activities-kill~ (~C-x C-a C-k~) :: Alias for ~activities-suspend~.
++ ~activities-new~ (~C-x C-a C-n~) :: Define a new activity whose default state is the current frame's or tab's window configuration.  With prefix argument, overwrite an existing activity (thereby updating its default state to the current state).
++ ~activities-suspend~ (~C-x C-a C-s~) :: Save an activity's state and close its frame or tab.
++ ~activities-kill~ (~C-x C-a C-k~) :: Discard an activity's last state (so when it is resumed, its default state will be used), and close its frame or tab.
 + ~activities-resume~ (~C-x C-a C-a~) :: Resume an activity, switching to a new frame or tab for its window configuration, and restoring its buffers.  With prefix argument, restore its default state rather than its last.
 + ~activities-revert~ (~C-x C-a g~) :: Revert an activity to its default state.
 + ~activities-switch~ (~C-x C-a RET~) :: Switch to an already-active activity.
@@ -129,6 +138,23 @@ When option ~activities-bookmark-store~ is enabled, an Emacs bookmark is stored
 
 * Changelog
 
+** v0.5
+
+*Additions*
++ Suggest setting variable ~edebug-inhibit-emacs-lisp-mode-bindings~ to avoid conflicts with suggested keybindings.
++ Option ~activities-bookmark-warnings~ enables warning messages when a non-file-visiting buffer can't be bookmarked (for debugging purposes).
++ Option ~activities-resume-into-frame~ controls whether resuming an activity opens a new frame or uses the current one (when ~activities-tabs-mode~ is disabled).  ([[https://github.com/alphapapa/activities.el/issues/22][#22]].  Thanks to [[https://github.com/Icy-Thought][Icy-Thought]] for suggesting.)
+
+*Changes*
++ Command ~activities-kill~ now discards an activity's last state (while ~activities-suspend~ saves its last state), and closes its frame or tab.
++ Face ~activities-tabs-face~ is renamed to ~activities-tabs~, and now inherits from another face by default, which allows it to adjust with the loaded theme.  ([[https://github.com/alphapapa/activities.el/issues/24][#24]].  Thanks to [[https://github.com/karthink][Karthik Chikmagalur]] for suggesting.)
+
+*Fixes*
++ Show a helpful error if a bookmark's target file is missing.  ([[https://github.com/alphapapa/activities.el/issues/17][#17]].  Thanks to [[https://github.com/jdtsmith][JD Smith]] for reporting.)
++ Sort order in ~activities-list~.
++ When discarding an inactive activity, don't switch to it first.  ([[https://github.com/alphapapa/activity.el/issues/18][#18]].  Thanks to [[https://github.com/jdtsmith][JD Smith]] for reporting.)
++ Don't signal an error when ~debug-on-error~ is enabled and a buffer is not visiting a file.  ([[https://github.com/alphapapa/activity.el/issues/25][#25]].  Thanks to [[https://github.com/karthink][Karthik Chikmagalur]] for reporting.)
+
 ** v0.4
 
 *Additions*

activities-list.el

@@ -75,7 +75,7 @@
            :formatter activities-list--format-time))
        :objects-function (lambda ()
                            (map-values activities-activities))
-       :sort-by '((2 . ascend) (0 . descend))
+       :sort-by '((2 . descend) (0 . descend))
        :actions `("q" (lambda (&rest _) (bury-buffer))
                   "n" (lambda (&rest _) (forward-line 1))
                   "p" (lambda (&rest _) (forward-line -1))

activities-tabs.el

@@ -50,8 +50,8 @@ Each is called with one argument, the activity."
   "Prepended to activity names in tabs."
   :type 'string)
 
-(defface activities-tabs-face
-  `((t :foreground ,(face-foreground 'font-lock-function-name-face nil 'default)))
+(defface activities-tabs
+  `((t :inherit font-lock-function-name-face))
   "Applied to tab-bar faces for tabs representing activities.")
 
 ;;;; Mode
@@ -126,11 +126,12 @@ Selects its tab, making one if needed.  Its state is not changed."
 
 (defun activities-tabs--tab-bar-tab-face-function (tab)
   "Return a face for TAB.
-If TAB represents an activity, `activities-tabs-face' is added."
+If TAB represents an activity, face `activities-tabs' is added as
+inherited."
   ;; TODO: Propose a tab-bar equivalent of `tab-line-tab-face-functions'.
   (let ((face (funcall activities-tabs-tab-bar-tab-face-function-original tab)))
     (if (activities-tabs--tab-parameter 'activity tab)
-        `(:inherit (activities-tabs-face ,face))
+        `(:inherit (activities-tabs ,face))
       face)))
 
 (defun activities-tabs-activity--set (activity)

activities.el

@@ -6,7 +6,7 @@
 ;; Maintainer: Adam Porter <[email protected]>
 ;; URL: https://github.com/alphapapa/activities.el
 ;; Keywords: convenience
-;; Version: 0.4
+;; Version: 0.5
 ;; Package-Requires: ((emacs "29.1") (persist "0.6"))
 
 ;; This program is free software; you can redistribute it and/or modify
@@ -284,6 +284,23 @@ non-nil, the activity's state is not saved."
               (function-item activities--backtrace-visible-p)
               (function :tag "Other predicate")))
 
+(defcustom activities-resume-into-frame 'current
+  "Which frame to resume an activity into.
+Only applies when `activities-tabs-mode' is disabled."
+  :type '(choice (const :tag "Current frame"
+                        :doc "Replace current frame's window configuration"
+                        current)
+                 (const :tag "New frame" :doc "Make a new frame" new)))
+
+(defcustom activities-bookmark-warnings nil
+  "Warn when a buffer can't be bookmarked.
+This is expected to be the case for non-file-visiting buffers
+whose major mode does not provide bookmark support, for which no
+warning is necessary.  This option may be enabled for debugging,
+which will cause a message to be printed for such buffers when an
+activity's state is saved."
+  :type 'boolean)
+
 ;;;; Commands
 
 ;;;###autoload
@@ -314,7 +331,9 @@ activity."
 If RESETP (interactively, with universal prefix), reset to
 ACTIVITY's default state; otherwise, resume its last state, if
 available."
-  (interactive (list (activities-completing-read) :resetp current-prefix-arg))
+  (interactive
+   (list (activities-completing-read :prompt "Resume activity" :default nil)
+         :resetp current-prefix-arg))
   (let ((already-active-p (activities-activity-active-p activity)))
     (activities--switch activity)
     (unless (or resetp already-active-p)
@@ -326,24 +345,32 @@ Interactively, offers active activities."
   (interactive
    (list (activities-completing-read
           :activities (cl-remove-if-not #'activities-activity-active-p activities-activities :key #'cdr)
-          :prompt "Switch to: ")))
+          :prompt "Switch to activity")))
   (activities--switch activity))
 
 (defun activities-suspend (activity)
   "Suspend ACTIVITY.
-Its last is saved, and its frames, windows, and tabs are closed."
+Its last state is saved, and its frames, windows, and tabs are closed."
   (interactive
-   (let ((default (when (activities-current)
-                    (activities-activity-name (activities-current)))))
-     (list (activities-completing-read
-            :activities (cl-remove-if-not #'activities-activity-active-p
-                                          activities-activities :key #'cdr)
-            :prompt (format-prompt "Suspend activity" default)
-            :default default))))
+   (list (activities-completing-read
+          :activities (cl-remove-if-not #'activities-activity-active-p
+                                        activities-activities :key #'cdr)
+          :prompt "Suspend activity")))
   (activities-save activity :lastp t)
   (activities-close activity))
 
-(defalias 'activities-kill #'activities-suspend)
+(defun activities-kill (activity)
+  "Kill ACTIVITY.
+Its last state is discarded (so when it is resumed, its default
+state will be used), and close its frame or tab."
+  (interactive
+   (list (activities-completing-read
+          :activities (cl-remove-if-not #'activities-activity-active-p
+                                        activities-activities :key #'cdr)
+          :prompt "Kill activity")))
+  (setf (activities-activity-last activity) nil)
+  (activities-save activity)
+  (activities-close activity))
 
 (defun activities-save-all ()
   "Save all active activities' last states.
@@ -366,14 +393,13 @@ In order to be safe for `kill-emacs-hook', this demotes errors."
 It will not be recoverable."
   ;; TODO: Discard relevant bookmarks when `activities-bookmark-store' is enabled.
   (interactive
-   (let ((default (when (activities-current)
-                    (activities-activity-name (activities-current)))))
-     (list (activities-completing-read :prompt (format-prompt "Discard activity" default)
-                                       :default default))))
-  (ignore-errors
-    ;; FIXME: After fixing all the bugs, remove ignore-errors.
-    (activities-close activity))
-  (setf activities-activities (map-delete activities-activities (activities-activity-name activity))))
+   (list (activities-completing-read :prompt "Discard activity")))
+  (when (yes-or-no-p (format "Discard activity %S permanently?" (activities-activity-name activity)))
+    (ignore-errors
+      ;; FIXME: After fixing all the bugs, remove ignore-errors.
+      (when (activities-activity-active-p activity)
+        (activities-close activity)))
+    (setf activities-activities (map-delete activities-activities (activities-activity-name activity)))))
 
 ;;;; Activity mode
 
@@ -419,17 +445,16 @@ To be called from `kill-emacs-hook'."
 If DEFAULTP, save its default state; if LASTP, its last.  If
 PERSISTP, force persisting of data (otherwise, data is persisted
 according to option `activities-always-persist', which see)."
-  (unless (or defaultp lastp)
-    (user-error "Neither DEFAULTP nor LASTP specified"))
   (activities-with activity
-    ;; Don't try to save if a minibuffer is active, because we
-    ;; wouldn't want to try to restore that layout.
-    (unless (run-hook-with-args-until-success 'activities-anti-save-predicates)
-      (pcase-let* (((cl-struct activities-activity name default last) activity)
-                   (new-state (activities-state)))
-        (setf (activities-activity-default activity) (if (or defaultp (not default)) new-state default)
-              (activities-activity-last activity) (if (or lastp (not last)) new-state last)
-              (map-elt activities-activities name) activity))))
+    (when (or defaultp lastp)
+      (unless (run-hook-with-args-until-success 'activities-anti-save-predicates)
+        (pcase-let* (((cl-struct activities-activity default last) activity)
+                     (new-state (activities-state)))
+          (setf (activities-activity-default activity) (if (or defaultp (not default)) new-state default)
+                (activities-activity-last activity) (if (or lastp (not last)) new-state last)))))
+    ;; Always set the value so, e.g. the activity can be modified
+    ;; externally and saved here.
+    (setf (map-elt activities-activities (activities-activity-name activity)) activity))
   (activities--persist persistp))
 
 (cl-defun activities-set (activity &key (state 'last))
@@ -475,8 +500,11 @@ closed."
   "Switch to ACTIVITY.
 Select's ACTIVITY's frame, making a new one if needed.  Its state
 is not changed."
-  (select-frame (or (activities--frame activity)
-                    (make-frame `((activity . ,activity)))))
+  (if (activities-activity-active-p activity)
+      (select-frame (activities--frame activity))
+    (pcase activities-resume-into-frame
+      ('current nil)
+      ('new (select-frame (make-frame `((activity . ,activity)))))))
   (unless activities-saving-p
     ;; HACK: Don't raise the frame when saving the activity's state.
     ;; (I don't love this solution, largely because it only applies
@@ -603,12 +631,13 @@ activity's name is NAME."
   "Return `activities-buffer' struct for BUFFER."
   (with-current-buffer buffer
     (make-activities-buffer
-     :bookmark (condition-case-unless-debug err
+     :bookmark (condition-case err
                    (bookmark-make-record)
                  (error
                   (pcase (error-message-string err)
                     ("Buffer not visiting a file or directory")
-                    (_ (message (format "Activities: Error while making bookmark for buffer %S: %%S" buffer) err)))
+                    (_ (when activities-bookmark-warnings
+                         (message (format "Activities: Error while making bookmark for buffer %S: %%S" buffer) err))))
                   nil))
      :filename (buffer-file-name buffer)
      :name (buffer-name buffer)
@@ -642,18 +671,49 @@ activity's name is NAME."
   ;; back (which actually break the entire bookmark system when
   ;; such a props is saved in the bookmarks file), we have to
   ;; workaround a failure to read here.  See bug#56643.
-  (pcase-let* (((cl-struct activities-buffer bookmark) struct))
-    (save-window-excursion
-      (condition-case err
-          (progn
-            (bookmark-jump bookmark)
-            (when-let ((local-variable-map
-                        (bookmark-prop-get bookmark 'activities-buffer-local-variables)))
-              (cl-loop for (variable . value) in local-variable-map
-                       do (setf (buffer-local-value variable (current-buffer)) value))))
-        (error (delay-warning 'activity
-                              (format "Error while opening bookmark: ERROR:%S  RECORD:%S" err struct))))
-      (current-buffer))))
+
+  ;; Unfortunately, when a bookmarked file no longer exists,
+  ;; `bookmark-handle-bookmark' handles the error itself and returns
+  ;; nil, preventing us from handling the error.  Since
+  ;; `bookmark-jump' works by side effect, we have to test whether the
+  ;; buffer was changed in order to know whether it worked.  We call
+  ;; it from a temp buffer in case the jumped-to buffer would be the
+  ;; same as the current buffer.
+  (with-temp-buffer
+    (pcase-let* (((cl-struct activities-buffer bookmark) struct)
+                 (temp-buffer (current-buffer))
+                 (jumped-to-buffer
+                  (save-window-excursion
+                    (condition-case err
+                        (progn
+                          (bookmark-jump bookmark)
+                          (when-let ((local-variable-map
+                                      (bookmark-prop-get bookmark 'activities-buffer-local-variables)))
+                            (cl-loop for (variable . value) in local-variable-map
+                                     do (setf (buffer-local-value variable (current-buffer)) value))))
+                      (error (delay-warning 'activity
+                                            (format "Error while opening bookmark: ERROR:%S  RECORD:%S" err struct))))
+                    (current-buffer))))
+      (if (not (eq temp-buffer jumped-to-buffer))
+          ;; Bookmark appears to have been jumped to: return that buffer.
+          jumped-to-buffer
+        ;; Bookmark appears to have not changed the buffer: return a new one showing an error.
+        (activities--error-buffer
+         (format "%s:%s" (car bookmark) (bookmark-prop-get bookmark 'filename))
+         (list "Activities was unable to get a buffer for bookmark:\n\n"
+	       (prin1-to-string bookmark) "\n\n"
+	       "It's likely that the bookmark's file no longer exists, in which case you may need to relocate it and redefine this activity.\n\n"
+               "If this is not the case, please report this error to the `activities' maintainer.\n\n"
+               "In the meantime, you may ignore this error and use the other buffers in the activity.\n\n"))))))
+
+(defun activities--error-buffer (name strings)
+  "Return a new error buffer having NAME and content STRINGS."
+  ;; TODO: Use this elsewhere too.
+  (with-current-buffer (get-buffer-create (format "*Activities (error): %s*" name))
+    (visual-line-mode)
+    (goto-char (point-max))
+    (apply #'insert strings)
+    (current-buffer)))
 
 (defun activities--filename-buffer (activities-buffer)
   "Return buffer for ACTIVITIES-BUFFER having `filename' set."
@@ -672,10 +732,15 @@ activity's name is NAME."
           (current-buffer)))))
 
 (cl-defun activities-completing-read
-    (&key (activities activities-activities) (prompt "Open activity: ") default)
+    (&key (activities activities-activities)
+          (default (when (activities-current)
+                     (activities-activity-name (activities-current))))
+          (prompt "Activity"))
   "Return an activity read with completion from ACTIVITIES.
-PROMPT and DEFAULT are passed to `completing-read', which see."
-  (let* ((names (activities-names activities))
+PROMPT is passed to `completing-read' by way of `format-prompt',
+which see, with DEFAULT."
+  (let* ((prompt (format-prompt prompt default))
+         (names (activities-names activities))
          (name (completing-read prompt names nil t nil 'activities-completing-read-history default)))
     (or (map-elt activities-activities name)
         (make-activities-activity :name name))))