Formalized the mode-line with a clickable menu to resume activities. … #103

shipmints · 2024-06-20T18:08:12Z

…Added a main menu-bar menu with all major commands.

No option to suppress the menu.

No work done on custom faces for the mode line.

No option to customize the mode-line appearance.

No work done on a context menu (not sure it's needed).

…Added a main menu-bar menu with all major commands. No option to suppress the menu. No work done on custom faces for the mode line. No option to customize the mode-line appearance. No work done on a context menu (not sure it's needed).

alphapapa · 2024-06-22T05:00:12Z

I appreciate your wanting to contribute substantially to this package. I don't recall having discussed copyright assignment yet, though. https://github.com/alphapapa/activities.el#copyright-assignment Have you done that?

shipmints · 2024-06-22T13:22:09Z

Yes.

alphapapa · 2024-06-22T14:55:44Z

Very well, but I am not allowed to take your word for it. The Emacs maintainers require me to verify by checking with them. What name and email address should I ask them about?

alphapapa · 2024-06-22T15:00:38Z

Yes.

I happened to just notice this post on emacs-devel by you: https://lists.gnu.org/archive/html/emacs-devel/2024-06/msg00691.html You have asked to begin the process, but you have not completed it, and I cannot accept your contributions until the process has been completed, and until I receive confirmation from the FSF secretary or the Emacs maintainers.

shipmints · 2024-06-22T15:05:07Z

Perfect. How will you get that confirmation or should I copy/paste their response to me?

alphapapa · 2024-06-22T15:06:04Z

When you send the form to the FSF, you can ask them to cc me on the confirmation. Otherwise, you can tell me that the process is completed, and then I can ask the maintainers to confirm.

shipmints · 2024-06-22T15:12:22Z

I've asked them to cc you and pasted your email address. Either way, I'll let you know. Thank you for your patience.

alphapapa

Thanks.

alphapapa · 2024-07-13T16:44:45Z

activities.el

+(defvar activities-mode-map (make-sparse-keymap)
+  "The mode keymap for `activities-mode'.")


Strictly speaking, I'm not sure if this is necessary, or actually correct, because it's not necessary to use activities-mode to use the various commands, so I'm not sure if the menu bar entries should be tied to a mode/map.

We need the mode's keymap for the menu to appear on the menu bar. If the mode isn't active, the menu won't appear so no harm. It doesn't affect general operation.

alphapapa · 2024-07-13T16:48:53Z

activities.el

+(defun activities-mode-line-format ()
+  (when-let ((cur (activities-current)))
+    (let ((cur-activity-title (concat " " (activities-name-for cur))))
+      `(:propertize
+        ,cur-activity-title
+        mouse-face mode-line-highlight
+        help-echo
+        ,(lambda (&rest _)
+           (concat
+            (format "Current activity:%s\n" cur-activity-title)
+            "mouse-1: Display minor mode menu\n"
+            "mouse-2: Show help for minor mode"))
+        keymap
+        ,(let ((map (make-sparse-keymap)))
+           (define-key map [mode-line down-mouse-1]
+                       activities-mode-line-menu)
+           (define-key map [mode-line down-mouse-3]
+                       activities-mode-line-menu)
+           (define-key map [mode-line mouse-2]
+                       (lambda ()
+                         (interactive)
+                         (describe-function 'activities-mode)))
+           map)))))


Nitpick, but I prefer not to abbreviate variable names in most cases, e.g. cur.

Probably the keymap should be defined statically.

I guess the lambda could be defined statically as well, i.e. not be a closure over the title variable, because it should be able to get the current activity name by doing what this function does when necessary.

The function should have a docstring. :)

Done. I left the closure and keymap as is since that seems to be the general approach via informal surveillance (and it works fine and without any overall performance issues).

alphapapa · 2024-07-13T16:50:10Z

activities.el

+(defcustom activities-mode-line '(:eval (activities-mode-line-format))
+  "Activities mode line definition."
+  :type 'sexp
+  :group 'activities
+  :risky t)


I think this should be called activities-mode-lighter, unless I'm missing something.

It shouldn't be necessary to specify the group, since it comes after the defcustom earlier in the file, right?

alphapapa · 2024-07-13T16:51:39Z

activities.el

  :global t
  :group 'activities
+  :lighter activities-mode-line
+  :keymap activities-mode-map


See earlier comment about menu bar and mode map.

Alternatively, we could consider defining this map as a prefix map for the suggested bindings.

Let's address prefix map and suggested bindings separately.

alphapapa · 2024-07-13T16:51:57Z

activities.el

      (setf activities-mode-timer nil))
    (remove-hook 'kill-emacs-hook #'activities-mode--killing-emacs)))

+(require 'easymenu)


It would be good to add a header comment for this section.

alphapapa · 2024-07-13T16:52:58Z

activities.el

+(setq activities-menu-item-resume
+      '("Resume..."
+        :help "Resume an existing activity"
+        :filter (lambda (&optional _)
+                  (let ((current-activity-name
+                         (when-let ((current-activity (activities-current)))
+                           (activities-activity-name current-activity))))
+                    (mapcar (lambda (act)
+                              (vector act `(activities-resume (activities-named ,act))
+                                      :style 'radio
+                                      :selected (equal current-activity-name act)
+                                      ))
+                            (activities-names))))
+        ))


I'm not sure what this is about, but a bare setq on an undefined variable is bogus, and there are hanging parens. :)

Done. Now a defvar.

alphapapa · 2024-07-13T16:54:01Z

activities.el

+(easy-menu-define activities-menu activities-mode-map
+  "Activities Menu"
+  `("Activities" :visible activities-mode
+    ["New" activities-new
+     :help "Create a new, empty activity"]
+    ["Define" activities-define
+     :help "Create a new activity using the current frame/tab"]
+    ["Resume" activities-resume
+     :help "Resume an existing activity"]
+    ,activities-menu-item-resume
+    ["Revert" activities-revert
+     :help "Revert the current activity to its original state"]
+    ["Suspend" activities-suspend
+     :help "Suspend the specified live activity and save its current state"]
+    ["Kill" activities-kill
+     :help "Revert and suspend the specified live activity"]
+    ["Switch" activities-switch
+     :help "Focus on the specified live activity"]
+    ["Switch Buffer" activities-switch-buffer
+     :help "Focus on the specified buffer from a live activity (activities-tabs-mode only)"]
+    ["List" activities-list
+     :help "Show the master list of known activities"]
+    ["Rename" activities-rename
+     :help "Rename the specified activity"]
+    ["Discard" activities-discard
+     :help "Discard the specified activity (this is undoable)"]
+    ["Save All" activities-save-all
+     :help "Save all live activities to disk"]
+    ))


It would be helpful to roughly organize these entries by purpose, with some separators between major types.

Also, hanging parens. :)

alphapapa · 2024-07-13T16:54:19Z

activities.el

+  "Activities Mode Line Menu"
+  `("Activities"
+    ,activities-menu-item-resume
+    ))


Also, hanging parens. :)

…nd-menu

…imer set.

shipmints · 2024-08-21T19:40:38Z

FYI, I added a defensive test in the mode before setting the timer, should someone set activities-mode-idle-frequency to nil (as I might).

shipmints mentioned this pull request Jun 20, 2024

Show the name of the current activity in an associated buffer's mode-line #97

Open

alphapapa requested changes Jul 13, 2024

View reviewed changes

alphapapa self-assigned this Jul 13, 2024

alphapapa added the enhancement New feature or request label Jul 13, 2024

alphapapa added this to the v0.8 milestone Jul 13, 2024

shipmints added 2 commits August 21, 2024 11:17

Merge remote-tracking branch 'origin/master' into mode-line-lighter-a…

e3efb84

…nd-menu

Addressing general comments. Wrapped activities-mode-idle-frequency t…

b3ff4b8

…imer set.

alphapapa added FSF CA confirmed priority:A labels Oct 17, 2024

		(defvar activities-mode-map (make-sparse-keymap)
		"The mode keymap for `activities-mode'.")

Formalized the mode-line with a clickable menu to resume activities. … #103

Are you sure you want to change the base?

Formalized the mode-line with a clickable menu to resume activities. … #103

Uh oh!

Conversation

shipmints commented Jun 20, 2024

Uh oh!

alphapapa commented Jun 22, 2024

Uh oh!

shipmints commented Jun 22, 2024

Uh oh!

alphapapa commented Jun 22, 2024

Uh oh!

alphapapa commented Jun 22, 2024

Uh oh!

shipmints commented Jun 22, 2024

Uh oh!

alphapapa commented Jun 22, 2024

Uh oh!

shipmints commented Jun 22, 2024

Uh oh!

alphapapa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shipmints commented Aug 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants