diff options
| -rw-r--r-- | pearl.el | 115 | ||||
| -rw-r--r-- | tests/test-pearl-adhoc.el | 114 | ||||
| -rw-r--r-- | tests/test-pearl-save.el | 8 |
3 files changed, 158 insertions, 79 deletions
@@ -3217,9 +3217,10 @@ active file with the query recorded as the source." (completing-read "Saved query: " (pearl--completion-table-keep-order - (pearl--with-none (mapcar #'car pearl-saved-queries))) + (pearl--with-sentinel pearl--filter-cancel + (mapcar #'car pearl-saved-queries))) nil t)))) - (when (pearl--filter-none-value-p name) + (when (pearl--filter-sentinel-value-p name) (user-error "Cancelled; no saved query run")) (let ((entry (assoc name pearl-saved-queries))) (unless entry @@ -3347,32 +3348,57 @@ with just what the user set." (when project (list :project project)) (when labels (list :labels labels)))) -(defconst pearl--filter-none "[ None. ]" - "Sentinel candidate shown at the top of `pearl--read-filter-interactively' -prompts. Picking it drops the dimension from the filter, the same as -opting out. Always rendered first so \"no constraint\" is a visible -choice rather than a hidden empty-input idiom.") - -(defun pearl--filter-none-value-p (value) - "Return non-nil when VALUE means \"no constraint\" for a filter dimension. -A nil, an empty string, or `pearl--filter-none' all mean the user opted -out of constraining this dimension." +(defconst pearl--filter-any "[ Any. ]" + "Sentinel candidate shown at the top of filter-dimension prompts. +Picking it means \"no constraint on this dimension\" -- i.e. *any* value +matches. Earlier drafts read \"[ None. ]\", which described what the user +picked rather than what the filter does; the dimension's not absent, it's +unconstrained. Always rendered first so the choice is visible rather +than a hidden empty-input idiom.") + +(defconst pearl--filter-cancel "[ Cancel. ]" + "Sentinel candidate shown at the top of pick-an-existing-thing prompts +(delete a saved query, run a saved query) where the opt-out reads as +\"don't do this\" rather than \"no constraint.\" Cancelling these +prompts is a real choice, not a missing one.") + +(defun pearl--filter-sentinel-value-p (value) + "Return non-nil when VALUE is a sentinel (any opt-out marker) or empty. +A nil, an empty string, `pearl--filter-any', or `pearl--filter-cancel' +all mean the user opted out (of constraining, or of acting)." (or (null value) (and (stringp value) (or (string-empty-p value) - (string= value pearl--filter-none))))) - -(defun pearl--with-none (candidates) - "Return CANDIDATES with `pearl--filter-none' prepended as the first option." - (cons pearl--filter-none candidates)) + (string= value pearl--filter-any) + (string= value pearl--filter-cancel))))) + +(defun pearl--with-sentinel (sentinel candidates) + "Return CANDIDATES with SENTINEL prepended as the first option." + (cons sentinel candidates)) + +(defun pearl--read-yes-no (prompt &optional default) + "Ask PROMPT as a completing-read over (\"yes\" \"no\") and return t / nil. +DEFAULT is the value RET picks without typing; \"yes\" if omitted. The +candidate list is ordered with the default first so the most-common +choice is what the user sees at the top, consistent with +`pearl--with-sentinel'. Use this for non-destructive prompts where +RET-through the defaults is a feature; keep `yes-or-no-p' for destructive +prompts (delete, overwrite) where typing \"yes\" is a safety affordance." + (let* ((d (or default "yes")) + (other (if (string= d "yes") "no" "yes")) + (choice (completing-read prompt + (pearl--completion-table-keep-order + (list d other)) + nil t nil nil d))) + (string= "yes" choice))) (defun pearl--completion-table-keep-order (candidates) "Return a completion table over CANDIDATES that preserves input order. The framework-side sort (vertico-sort-function, prescient, ivy, etc.) -re-sorts alphabetically by default, which clobbers `pearl--filter-none' -as the topmost option whenever a real candidate sorts ahead of \"[\". -Setting the table's `display-sort-function' to `identity' is the standard -Emacs hook for \"these are pre-sorted, leave them alone\"." +re-sorts alphabetically by default, which clobbers the sentinel as the +topmost option whenever a real candidate sorts ahead of \"[\". Setting +the table's `display-sort-function' to `identity' is the standard Emacs +hook for \"these are pre-sorted, leave them alone\"." (lambda (string pred action) (if (eq action 'metadata) '(metadata (display-sort-function . identity) @@ -3382,56 +3408,56 @@ Emacs hook for \"these are pre-sorted, leave them alone\"." (defun pearl--read-filter-interactively () "Build a filter plist by completing over the chosen team's fetched dimensions. Picks a team first (scoping the rest), then offers open-only, state, project, -labels, and assignee. Each prompt carries `pearl--filter-none' as the topmost -choice; selecting it drops that dimension. The assignee prompt offers `me' -(the viewer), `member' (a specific teammate, resolved to an id), or -`pearl--filter-none' (no scoping)." +labels, and assignee. Each prompt carries `pearl--filter-any' as the topmost +choice; selecting it means \"no constraint on this dimension.\" The assignee +prompt offers `me' (the viewer), `member' (a specific teammate, resolved to +an id), or `pearl--filter-any' (any assignee, no scoping)." (let* ((teams (pearl--all-teams)) (team-name (completing-read "Team: " (pearl--completion-table-keep-order - (pearl--with-none + (pearl--with-sentinel pearl--filter-any (mapcar (lambda (tm) (cdr (assoc 'name tm))) teams))) nil t)) - (team-id (and (not (pearl--filter-none-value-p team-name)) + (team-id (and (not (pearl--filter-sentinel-value-p team-name)) (pearl--get-team-id-by-name team-name))) - (open (y-or-n-p "Open issues only? ")) + (open (pearl--read-yes-no "Open issues only? ")) (state (and team-id (let ((s (completing-read "State: " (pearl--completion-table-keep-order - (pearl--with-none + (pearl--with-sentinel pearl--filter-any (mapcar (lambda (st) (cdr (assoc 'name st))) (pearl--team-states team-id)))) nil t))) - (unless (pearl--filter-none-value-p s) s)))) + (unless (pearl--filter-sentinel-value-p s) s)))) (project (and team-id (let ((p (completing-read "Project: " (pearl--completion-table-keep-order - (pearl--with-none + (pearl--with-sentinel pearl--filter-any (pearl--team-collection-names 'projects team-id))) nil t))) ;; The :project filter compiles to project.id.eq, so ;; resolve the picked name to its id before passing. - (unless (pearl--filter-none-value-p p) + (unless (pearl--filter-sentinel-value-p p) (pearl--resolve-team-id 'projects p team-id))))) (labels (and team-id - ;; `[ None. ]' alongside real labels is redundant, so + ;; `[ Any. ]' alongside real labels is redundant, so ;; filter the sentinel out of the result. A result of - ;; only-sentinel (or empty) means "no labels." - (cl-remove pearl--filter-none + ;; only-sentinel (or empty) means "no label constraint." + (cl-remove pearl--filter-any (completing-read-multiple "Labels (comma-separated): " (pearl--completion-table-keep-order - (pearl--with-none + (pearl--with-sentinel pearl--filter-any (pearl--team-collection-names 'labels team-id))) nil t) :test #'string=))) (assignee (let ((choice (completing-read "Assignee: " (pearl--completion-table-keep-order - (list pearl--filter-none "me" "member")) + (list pearl--filter-any "me" "member")) nil t))) (pcase choice ("me" :me) @@ -3447,7 +3473,7 @@ choice; selecting it drops that dimension. The assignee prompt offers `me' ;; The :team filter compiles to team.key.eq, so look the picked name up ;; in the teams alist and pass its key (not the display name). (pearl--assemble-filter - (and (not (pearl--filter-none-value-p team-name)) + (and (not (pearl--filter-sentinel-value-p team-name)) (cdr (assoc 'key (cl-find team-name teams :key (lambda (tm) (cdr (assoc 'name tm))) @@ -3458,8 +3484,8 @@ choice; selecting it drops that dimension. The assignee prompt offers `me' (defun pearl-delete-saved-query (name) "Delete the saved query NAME from `pearl-saved-queries' after confirmation. Interactively, completes over the configured query names with -`pearl--filter-none' at the top as a cancel. Local-only: removes the entry -from the customized list and persists via `customize-save-variable'. Linear +`pearl--filter-cancel' at the top. Local-only: removes the entry from +the customized list and persists via `customize-save-variable'. Linear is untouched (saved queries are local; syncing them up is a separate spec)." (interactive (list (if (null pearl-saved-queries) @@ -3467,10 +3493,11 @@ is untouched (saved queries are local; syncing them up is a separate spec)." (completing-read "Delete saved query: " (pearl--completion-table-keep-order - (pearl--with-none (mapcar #'car pearl-saved-queries))) + (pearl--with-sentinel pearl--filter-cancel + (mapcar #'car pearl-saved-queries))) nil t)))) (cond - ((pearl--filter-none-value-p name) + ((pearl--filter-sentinel-value-p name) (message "Cancelled; no saved query deleted")) ((not (assoc name pearl-saved-queries)) (user-error "No saved query named %s" name)) @@ -3502,7 +3529,7 @@ offers to save the filter as a local query. FILTER-PLIST is the authoring filter; SAVE-NAME, when given, persists it via `pearl--save-query'." (interactive (list (pearl--read-filter-interactively) - (when (y-or-n-p "Save this filter locally so you can re-run it (not pushed to Linear)? ") + (when (pearl--read-yes-no "Save this filter locally so you can re-run it (not pushed to Linear)? ") (read-string "Name for this saved query: ")))) ;; Validate before saving or running, so a bad filter never gets persisted ;; or run -- the command boundary is where the clear error belongs. @@ -4457,7 +4484,7 @@ the prompt counts comments as viewer-unavailable rather than read-only. BUFFER is surfaced if anything pushed. Declining the prompt mutates nothing. The queue continues past a per-field conflict, so one conflicted issue does not stop the rest." - (if (not (y-or-n-p (pearl--save-all-prompt + (if (not (pearl--read-yes-no (pearl--save-all-prompt (pearl--save-all-counts scan viewer-id viewer-failed)))) (message "save-all cancelled; nothing saved") (let (thunks) diff --git a/tests/test-pearl-adhoc.el b/tests/test-pearl-adhoc.el index 2dec1a3..558578a 100644 --- a/tests/test-pearl-adhoc.el +++ b/tests/test-pearl-adhoc.el @@ -28,34 +28,43 @@ (require 'test-bootstrap (expand-file-name "test-bootstrap.el")) (require 'cl-lib) -;;; --filter-none sentinel helpers - -(ert-deftest test-pearl-filter-none-value-p-recognizes-sentinel () - "The sentinel string reads as a `no constraint' value." - (should (pearl--filter-none-value-p pearl--filter-none))) - -(ert-deftest test-pearl-filter-none-value-p-recognizes-empty-and-nil () - "Empty input and nil also read as `no constraint' (for back-compat)." - (should (pearl--filter-none-value-p "")) - (should (pearl--filter-none-value-p nil))) - -(ert-deftest test-pearl-filter-none-value-p-rejects-real-values () - "An actual picked value does not read as `no constraint'." - (should-not (pearl--filter-none-value-p "In Progress")) - (should-not (pearl--filter-none-value-p "Platform")) - (should-not (pearl--filter-none-value-p "bug"))) - -(ert-deftest test-pearl-with-none-prepends-sentinel () - "`pearl--with-none' puts the sentinel first so it sits at the top of the -completing-read candidate list." - (let ((wrapped (pearl--with-none '("alpha" "beta")))) - (should (string= pearl--filter-none (car wrapped))) - (should (equal '("alpha" "beta") (cdr wrapped))))) - -(ert-deftest test-pearl-with-none-empty-list-yields-just-the-sentinel () +;;; sentinel helpers + +(ert-deftest test-pearl-filter-sentinel-value-p-recognizes-any-and-cancel () + "Both opt-out sentinels read as sentinel values." + (should (pearl--filter-sentinel-value-p pearl--filter-any)) + (should (pearl--filter-sentinel-value-p pearl--filter-cancel))) + +(ert-deftest test-pearl-filter-sentinel-value-p-recognizes-empty-and-nil () + "Empty input and nil also read as opt-out (back-compat for the legacy +empty-input idiom and for callers that pass nil)." + (should (pearl--filter-sentinel-value-p "")) + (should (pearl--filter-sentinel-value-p nil))) + +(ert-deftest test-pearl-filter-sentinel-value-p-rejects-real-values () + "An actual picked value does not read as opt-out." + (should-not (pearl--filter-sentinel-value-p "In Progress")) + (should-not (pearl--filter-sentinel-value-p "Platform")) + (should-not (pearl--filter-sentinel-value-p "bug"))) + +(ert-deftest test-pearl-filter-any-and-cancel-are-distinct-strings () + "The two sentinels carry different labels so the prompt reads accurately +for its case: \"any\" for filter dimensions, \"cancel\" for pick-an-existing." + (should-not (string= pearl--filter-any pearl--filter-cancel))) + +(ert-deftest test-pearl-with-sentinel-prepends-the-given-sentinel () + "`pearl--with-sentinel' puts SENTINEL first so it sits at the top of the +candidate list." + (let ((wrapped-any (pearl--with-sentinel pearl--filter-any '("alpha" "beta")))) + (should (string= pearl--filter-any (car wrapped-any))) + (should (equal '("alpha" "beta") (cdr wrapped-any)))) + (let ((wrapped-cancel (pearl--with-sentinel pearl--filter-cancel '("a" "b")))) + (should (string= pearl--filter-cancel (car wrapped-cancel))))) + +(ert-deftest test-pearl-with-sentinel-empty-list-yields-just-the-sentinel () "An empty candidate list still yields a one-element list with the sentinel." - (let ((wrapped (pearl--with-none '()))) - (should (equal (list pearl--filter-none) wrapped)))) + (let ((wrapped (pearl--with-sentinel pearl--filter-any '()))) + (should (equal (list pearl--filter-any) wrapped)))) (ert-deftest test-pearl-completion-table-keep-order-metadata-pins-identity () "Action `metadata' returns an alist with `display-sort-function' bound to @@ -66,6 +75,49 @@ completing-read candidate list." (should (eq #'identity (alist-get 'display-sort-function (cdr meta)))) (should (eq #'identity (alist-get 'cycle-sort-function (cdr meta)))))) +(ert-deftest test-pearl-read-yes-no-returns-t-for-yes () + "A `yes' choice returns t." + (cl-letf (((symbol-function 'completing-read) (lambda (&rest _) "yes"))) + (should (eq t (pearl--read-yes-no "Anything? "))))) + +(ert-deftest test-pearl-read-yes-no-returns-nil-for-no () + "A `no' choice returns nil." + (cl-letf (((symbol-function 'completing-read) (lambda (&rest _) "no"))) + (should-not (pearl--read-yes-no "Anything? ")))) + +(ert-deftest test-pearl-read-yes-no-default-passed-as-default-arg () + "The DEFAULT argument is the value RET takes without typing; the helper +forwards it as completing-read's default so the framework highlights it." + (let (forwarded-default) + (cl-letf (((symbol-function 'completing-read) + (lambda (&rest args) (setq forwarded-default (nth 6 args)) "no"))) + (pearl--read-yes-no "?" "no") + (should (string= "no" forwarded-default))))) + +(ert-deftest test-pearl-read-yes-no-default-yes-orders-yes-first () + "With the default `yes' (the omitted-arg case), the candidate list is +ordered yes-then-no so the most-common choice sits at the top." + (let (captured-collection) + (cl-letf (((symbol-function 'completing-read) + (lambda (_p coll &rest _) + (setq captured-collection + (when (functionp coll) (all-completions "" coll))) + "yes"))) + (pearl--read-yes-no "?") + (should (equal '("yes" "no") captured-collection))))) + +(ert-deftest test-pearl-read-yes-no-default-no-orders-no-first () + "With DEFAULT \"no\", the candidate list is ordered no-then-yes so a +non-affirmative default is what the user lands on." + (let (captured-collection) + (cl-letf (((symbol-function 'completing-read) + (lambda (_p coll &rest _) + (setq captured-collection + (when (functionp coll) (all-completions "" coll))) + "no"))) + (pearl--read-yes-no "?" "no") + (should (equal '("no" "yes") captured-collection))))) + (ert-deftest test-pearl-completion-table-keep-order-delegates-all-completions () "Non-metadata actions delegate to `complete-with-action' over the original candidate list, so completion still works the way the framework expects." @@ -151,14 +203,14 @@ candidate list, so completion still works the way the framework expects." (should-not persisted)))) (ert-deftest test-pearl-delete-saved-query-sentinel-cancels () - "Picking the `pearl--filter-none' sentinel cancels without touching anything." + "Picking the `pearl--filter-cancel' sentinel cancels without touching anything." (let ((pearl-saved-queries '(("Keep" :filter (:open t)))) (confirmed nil)) (cl-letf (((symbol-function 'yes-or-no-p) (lambda (&rest _) (setq confirmed t) t)) ((symbol-function 'customize-save-variable) (lambda (&rest _) nil))) - (pearl-delete-saved-query pearl--filter-none) + (pearl-delete-saved-query pearl--filter-cancel) (should (assoc "Keep" pearl-saved-queries)) (should-not confirmed)))) @@ -177,12 +229,12 @@ rather than offering an empty picker." ;;; pearl-run-saved-query sentinel cancel (ert-deftest test-pearl-run-saved-query-sentinel-cancels () - "Picking the `pearl--filter-none' sentinel cancels the run without fetching." + "Picking the `pearl--filter-cancel' sentinel cancels the run without fetching." (let ((fetched nil) (pearl-saved-queries '(("Open" :filter (:open t))))) (cl-letf (((symbol-function 'pearl--query-issues-async) (lambda (&rest _) (setq fetched t)))) - (should-error (pearl-run-saved-query pearl--filter-none) :type 'user-error) + (should-error (pearl-run-saved-query pearl--filter-cancel) :type 'user-error) (should-not fetched)))) ;;; pearl-list-issues-filtered diff --git a/tests/test-pearl-save.el b/tests/test-pearl-save.el index 47b48c2..b82ca04 100644 --- a/tests/test-pearl-save.el +++ b/tests/test-pearl-save.el @@ -755,7 +755,7 @@ description hash is left untouched and the title still saved." (test-pearl-save--in-two-issues (let ((prompted nil) (fetched nil)) (test-pearl-save--with-net - (cl-letf (((symbol-function 'y-or-n-p) (lambda (&rest _) (setq prompted t) t)) + (cl-letf (((symbol-function 'pearl--read-yes-no) (lambda (&rest _) (setq prompted t) t)) ((symbol-function 'pearl--fetch-issue-description-async) (lambda (&rest _) (setq fetched t)))) (pearl-save-all) @@ -770,7 +770,7 @@ description hash is left untouched and the title still saved." (end-of-line) (insert " EDITED")) (goto-char (point-min)) (test-pearl-save--with-net - (cl-letf (((symbol-function 'y-or-n-p) (lambda (&rest _) t)) + (cl-letf (((symbol-function 'pearl--read-yes-no) (lambda (&rest _) t)) ((symbol-function 'pearl--fetch-issue-description-async) (lambda (_id cb) (funcall cb '(:description "Body." :updated-at "t0"))))) (pearl-save-all) @@ -783,7 +783,7 @@ description hash is left untouched and the title still saved." (end-of-line) (insert " EDITED")) (goto-char (point-min)) (test-pearl-save--with-net - (cl-letf (((symbol-function 'y-or-n-p) (lambda (&rest _) nil)) + (cl-letf (((symbol-function 'pearl--read-yes-no) (lambda (&rest _) nil)) ((symbol-function 'pearl--fetch-issue-description-async) (lambda (_id cb) (funcall cb '(:description "Body." :updated-at "t0"))))) (pearl-save-all) @@ -803,7 +803,7 @@ description hash is left untouched and the title still saved." (end-of-line) (insert " EDITED")) (goto-char (point-min)) (test-pearl-save--with-net - (cl-letf (((symbol-function 'y-or-n-p) (lambda (&rest _) t)) + (cl-letf (((symbol-function 'pearl--read-yes-no) (lambda (&rest _) t)) ((symbol-function 'pearl--fetch-issue-description-async) (lambda (_id cb) (funcall cb '(:description "Body." :updated-at "t0"))))) (pearl-save-all) |