feat - tag prompt using choice.el

It now makes no changes to the file, and only accepts two plists
instead of a tag list and a plist.

Cask

@@ -6,12 +6,13 @@
  "0.6.4"
  "A time tracker for Emacs with a nice interface")
 
-(depends-on "cl")
+(depends-on "cl-lib")
 (depends-on "dash"     "2.16.0")
 (depends-on "seq"      "2.20")
 (depends-on "s"        "1.12.0")
 (depends-on "ts"       "0.2")
 (depends-on "anaphora" "1.0.4")
+(depends-on "choice"   "0.1.0")
 
 (files "elisp/*.el")
 

TODO.org

@@ -336,6 +336,8 @@ ppp.el doesn't align plist values along the same column. It's also GPL, and I'm
       * Tags and key-values are optional extensions; we don't want Chronometrist to know about them.
         + Well, even with this style of configuration, Chronometrist doesn't necessarily have to...it could use the fields it knows about, ignoring the rest; the extensions could check for the fields they know about.
       * Instead of setting the prompt function, set hooks (=chronometrist-before-in-functions=/=chronometrist-after-in-functions=/=chronometrist-before-out-functions=/=chronometrist-after-out-functions=) per-task. This is preferable, because if you define a custom prompt function, you probably also want to remove certain functions coming earlier in the hook, such as =chronometrist-skip-query-prompt=, for that task.
+      * The real problem is that we want to use fewer keystrokes to access previous tags - how about a Hydra interface? Press keys 1-9 to select a set of tags from the recently used ones, or press e to edit the selection or enter new ones.
+        + Alternatively, press keys 1-9 to select/deselect one or more tags.
    2. =:hide= - values can be nil (the default) or t - if t, hide this task from being displayed in =chronometrist=/=chronometrist-report=/=chronometrist-statistics= buffers. (effectively a non-destructive deletion of all intervals of the task)
    ...
 

elisp/chronometrist-key-values.el

@@ -5,6 +5,7 @@
 (require 'dash)
 (require 'seq)
 (require 'anaphora)
+(require 'choice)
 
 (require 'chronometrist-migrate)
 (require 'chronometrist-events)
@@ -62,43 +63,52 @@ Used as history by `chronometrist--value-suggestions'.")
            it)
          list))
 
-(defun chronometrist-append-to-last (tags plist)
-  "Add TAGS and PLIST to the last entry in `chronometrist-file'.
-TAGS should be a list of symbols and/or strings.
+(defun chronometrist-plist-update (old-plist new-plist)
+  "Add tags and keyword-values from NEW-PLIST to OLD-PLIST.
+OLD-PLIST and NEW-PLIST should be a property lists.
 
-PLIST should be a property list. Properties reserved by
-Chronometrist - currently :name, :tags, :start, and :stop - will
-be removed."
-  (let* ((old-expr  (chronometrist-last))
-         (old-name  (plist-get old-expr :name))
-         (old-start (plist-get old-expr :start))
-         (old-stop  (plist-get old-expr :stop))
-         (old-tags  (plist-get old-expr :tags))
-         ;; Anything that's left will be the user's key-values.
-         (old-kvs   (chronometrist-plist-remove old-expr :name :tags :start :stop))
-         ;; Prevent the user from adding reserved key-values.
-         (plist     (chronometrist-plist-remove plist :name :tags :start :stop))
-         (new-tags  (if old-tags
-                        (-> (append old-tags tags)
-                            (cl-remove-duplicates :test #'equal))
-                      tags))
-         ;; In case there is an overlap in key-values, we use
-         ;; plist-put to replace old ones with new ones.
-         (new-kvs   (cl-copy-list old-expr))
-         (new-kvs   (if plist
-                        (-> (cl-loop for (key val) on plist by #'cddr
-                              do (plist-put new-kvs key val)
-                              finally return new-kvs)
-                            (chronometrist-plist-remove :name :tags :start :stop))
-                      old-kvs))
-         (plist     (append `(:name ,old-name)
-                            (when new-tags `(:tags ,new-tags))
-                            new-kvs
-                            `(:start ,old-start)
-                            (when old-stop `(:stop  ,old-stop)))))
-    (chronometrist-sexp-replace-last plist)))
+Keywords reserved by Chronometrist - :name, :start, and :stop -
+will not be updated. Keywords in OLD-PLIST with new values in
+NEW-PLIST will be updated. Tags in OLD-PLIST will be preserved
+alongside new tags from NEW-PLIST."
+  (-let* (((&plist :name  old-name  :tags old-tags
+                   :start old-start :stop old-stop) old-plist)
+          ;; Anything that's left will be the user's key-values.
+          (old-kvs   (chronometrist-plist-remove old-plist :name :tags :start :stop))
+          ;; Prevent the user from adding reserved key-values.
+          (plist     (chronometrist-plist-remove new-plist :name :tags :start :stop))
+          (new-tags  (-> (append old-tags (plist-get new-plist :tags))
+                         (cl-remove-duplicates :test #'equal)))
+          ;; In case there is an overlap in key-values, we use
+          ;; plist-put to replace old ones with new ones.
+          (new-kvs   (cl-copy-list old-plist))
+          (new-kvs   (if plist
+                         (-> (cl-loop for (key val) on plist by #'cddr
+                               do (plist-put new-kvs key val)
+                               finally return new-kvs)
+                             (chronometrist-plist-remove :name :tags :start :stop))
+                       old-kvs)))
+    (append `(:name ,old-name)
+            (when new-tags `(:tags ,new-tags))
+            new-kvs
+            `(:start ,old-start)
+            (when old-stop `(:stop  ,old-stop)))))
 
 ;;;; TAGS ;;;;
+(defcustom chronometrist-tag-history-style :combinations
+  "How previously-used tags are suggested.
+Valid values are :combinations and :individual."
+  :group 'chronometrist-key-values
+  :type '(choice (const :combinations)
+                 (const :individual)))
+
+(defcustom chronometrist-key-history-style :individual
+  "How previously-used tags are suggested.
+Valid values are :combinations and :individual."
+  :group 'chronometrist-key-values
+  :type '(choice (const :combinations)
+                 (const :individual)))
+
 (defvar chronometrist-tags-history (make-hash-table :test #'equal)
   "Hash table of tasks and past tag combinations.
 Each value is a list of tag combinations, in reverse
@@ -139,16 +149,19 @@ HISTORY-TABLE must be a hash table (see `chronometrist-key-history')."
   (puthash task nil history-table)
   (chronometrist-loop-file for plist in file do
     (catch 'quit
-      (let* ((name  (plist-get plist :name))
-             (check (unless (equal name task) (throw 'quit nil)))
-             (keys  (--> (chronometrist-plist-remove plist :name :start :stop :tags)
-                         (seq-filter #'keywordp it)
-                         (cl-loop for key in it collect
-                           (s-chop-prefix ":" (symbol-name key)))))
-             (check (unless keys (throw 'quit nil)))
+      (let* ((name     (plist-get plist :name))
+             (check    (unless (equal name task) (throw 'quit nil)))
+             (new-keys (--> (chronometrist-plist-remove plist :name :start :stop :tags)
+                            (seq-filter #'keywordp it)
+                            (cl-loop for key in it collect
+                              (s-chop-prefix ":" (symbol-name key)))))
+             (check    (unless new-keys (throw 'quit nil)))
+             (new-keys (case chronometrist-key-history-style
+                         (:combinations (list new-keys))
+                         (:individual   new-keys)))
              (old-keys (gethash name history-table)))
         (puthash name
-                 (if old-keys (append old-keys keys) keys)
+                 (if old-keys (append old-keys new-keys) new-keys)
                  history-table))))
   (chronometrist-history-prep task history-table))
 
@@ -232,23 +245,20 @@ INITIAL-INPUT is as used in `completing-read'."
   "Read tags from the user; add them to the last entry in `chronometrist-file'.
 _ARGS are ignored. This function always returns t, so it can be
 used in `chronometrist-before-out-functions'."
+  (interactive)
   (unless chronometrist--skip-detail-prompts
     (let* ((last-expr (chronometrist-last))
            (last-name (plist-get last-expr :name))
            (_history  (chronometrist-tags-history-populate last-name chronometrist-tags-history chronometrist-file))
            (last-tags (plist-get last-expr :tags))
-           (input     (->> last-tags
-                           (chronometrist-maybe-symbol-to-string)
+           (input     (->> (chronometrist-maybe-symbol-to-string last-tags)
                            (-interpose ",")
                            (apply #'concat)
                            (chronometrist-tags-prompt last-name)
                            (chronometrist-maybe-string-to-symbol))))
       (when input
-        (--> (append last-tags input)
-             (reverse it)
-             (cl-remove-duplicates it :test #'equal)
-             (reverse it)
-             (chronometrist-append-to-last it nil)))))
+        (--> (chronometrist-plist-update (chronometrist-sexp-last) (list :tags input))
+             (chronometrist-sexp-replace-last it)))))
   t)
 
 ;;;; KEY-VALUES ;;;;
@@ -402,8 +412,9 @@ used in `chronometrist-before-out-functions'."
       (goto-char (point-min))
       (setq user-kv-expr (ignore-errors (read (current-buffer))))
       (kill-buffer chronometrist-kv-buffer-name))
-    (if user-kv-expr
-        (chronometrist-append-to-last nil user-kv-expr)
+    (aif user-kv-expr
+        (chronometrist-sexp-replace-last
+         (chronometrist-plist-update (chronometrist-sexp-last) it))
       (chronometrist-refresh))))
 
 (defun chronometrist-kv-reject ()
@@ -431,7 +442,8 @@ This function always returns t, so it can be used in `chronometrist-before-out-f
          (yes-or-no-p
           (format "Skip prompt and use last-used tags/key-values? %S " plist))
          (setq chronometrist--skip-detail-prompts t)
-         (chronometrist-append-to-last (plist-get plist :tags) plist))
+         (chronometrist-sexp-replace-last
+          (chronometrist-plist-update (chronometrist-sexp-last) plist)))
     t))
 
 (defun chronometrist-skip-query-reset (_task)
@@ -439,6 +451,81 @@ This function always returns t, so it can be used in `chronometrist-before-out-f
 This function always returns t, so it can be used in `chronometrist-before-out-functions'."
   (setq chronometrist--skip-detail-prompts nil) t)
 
+;; TODO
+;; 1. rename `chronometrist-tags-history' to `chronometrist-tag-history' for consistency
+;; 2. suggest key combinations for task, instead of individual keys
+;;    * values for each of the selected keys can be queried one by one
+;;      after that
+;;    * make it possible to select new keys after initial
+;;      key-combination selection - perhaps at a confirmation step
+;;      after the values are selected?
+;; 3. select a combination and edit it
+;;    * use universal argument?
+;; 4. Multiple values for a key
+
+;; #### POSSIBLE INTERFACES ####
+;;
+;; (#1 and #2 are meant to be mixed and matched.)
+;;
+;; 1. (tag|key|value) combinations -> ...
+;;      0-9     - use combination (and exit)
+;;      C-u 0-9 - edit combination (then exit)
+;;      s       - skip (exit)
+;;      (b      - back [to previous prompt])
+
+;; 2. select individual (tags|keys|values) -> ...
+;;      0-9 - select keys (toggles; save in var; doesn't exit)
+;;      u   - use selection (and exit)
+;;      e   - edit selection (then exit)
+;;      s   - skip (exit)
+;;      (b  - back [to previous prompt])
+;;    Great for values; makes it easy to add multiple values, too,
+;;    especially for users who don't know Lisp.
+
+;; 3. tag-key-value combinations (everything in one prompt)
+;;      0-9     - use combination (and exit)
+;;      C-u 0-9 - edit combination (then exit)
+;;      s       - skip (exit)
+
+;; [x] we want C-g to quit, and universal arg to work...
+
+;; FIXME - incorrect tags added to file
+
+(defun chronometrist-defchoice (mode key table)
+  "MODE ::= :tag
+          | :key
+          | :value
+
+KEY ::= \"task\" (if MODE is :tags or :keys)
+      | \"key\" (if MODE is :values)"
+  (cl-loop with num = 0
+    for comb in (-take 10 (gethash key table))
+    do (incf num)
+    if (= num 10) do (setq num 0)
+    collect
+    (list (format "%s" num)
+          `(chronometrist-sexp-replace-last
+            (chronometrist-plist-update (chronometrist-sexp-last) ',(list :tags comb)))
+          (format "%s" comb))
+    into numeric-commands
+    finally do
+    (eval `(defchoice ,(intern
+                        (format
+                         "chronometrist-%s" (s-chop-prefix ":" (symbol-name mode))))
+             ,@numeric-commands
+             ("s" nil "skip")))))
+
+(defun chronometrist-tag-choice (task)
+  "Query user for tags to be added to TASK.
+Return t, to permit use in `chronometrist-before-out-functions'."
+  (let ((table chronometrist-tags-history))
+    (chronometrist-tags-history-populate task table chronometrist-file)
+    (if (hash-table-empty-p table)
+        (chronometrist-tags-add)
+      (chronometrist-defchoice :tag task table)
+      (chronometrist-tag-choice-prompt "Which tags?"))
+    t))
+
 (provide 'chronometrist-key-values)
 
 ;;; chronometrist-key-values.el ends here

tests-ert/key-value-tests.el

@@ -29,6 +29,30 @@
    (equal (chronometrist-plist-remove '(:a 1 :b 2 :c 3 :d 4) :d :a)
           '(:b 2 :c 3))))
 
+(ert-deftest chronometrist-plist-update ()
+  (let ((test-plist-1 '(:name "Old name"
+                              :tags (foo)
+                              :key1 "val 1"
+                              :start "2021-01-10T22:59:23+0530"
+                              :stop "2021-01-10T22:59:27+0530"))
+        (test-plist-2 '(:name "New name" :tags (bar)
+                              :key1 "new val 1"
+                              :key2 "val 2"
+                              :start "2021-01-10T22:59:23+0530"
+                              :stop "2021-01-10T22:59:27+0530")))
+    ;; :name, :start, and :stop should not be updated
+    ;; same keys should be updated
+    ;; new keys should be added
+    ;; old tags should be preserved
+    ;; new tags should be added
+    (should (equal (chronometrist-plist-update test-plist-1 test-plist-2)
+                   '(:name "Old name"
+                           :tags (foo bar)
+                           :key1 "new val 1"
+                           :key2 "val 2"
+                           :start "2021-01-10T22:59:23+0530"
+                           :stop "2021-01-10T22:59:27+0530")))))
+
 (ert-deftest chronometrist-tags-history ()
   (progn
     (clrhash chronometrist-tags-history)