Skip to content

Commit 0c32dbd

Browse files
committed
gptel: Improve preset validation, modify-spec handling
* gptel.el (gptel--modify-value): Use keywordp instead of checking for it the hard way. I keep forgetting that keywordp exists. (Like car-safe, this is a weird choice of functions to implement in C.) * gptel-transient.el (gptel--preset-mismatch-p, gptel--read-apply-preset): Set `gptel--preset' when applying a preset via `completing-read'. Try to handle modify-specs for :tools when checking if there is a preset mismatch. Handle both tools and tool names in the :tools spec correctly. (#1099) Handling all possible modify-specs correctly in `gptel--preset-mismatch-p' is impossible, or at least impractical. For example, if a spec contains :system (:function 'do-something) where do-something is not idempotent, then we have nothing to compare the current system message (which is (do-something gptel--system-message)) against gptel--system-message.
1 parent 693a12a commit 0c32dbd

File tree

2 files changed

+16
-11
lines changed

2 files changed

+16
-11
lines changed

gptel-transient.el

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ For internal use only.")
8989
((memq key '(:description :parents)) 'nil)
9090
((eq key :system)
9191
(or (equal gptel--system-message val)
92+
(functionp val) ; Ignore functions, modify-specs for speed here
93+
(and (consp val) (keywordp (car val)))
9294
(and-let* (((symbolp val))
9395
(p (assq val gptel-directives)))
9496
(equal gptel--system-message (cdr p)))
@@ -99,18 +101,22 @@ For internal use only.")
99101
(eq gptel-backend val))
100102
(throw 'mismatch t)))
101103
((eq key :tools)
102-
(if (eq (car-safe val) :append)
103-
(cl-loop for name in (cdr val) ;preset tools contained in gptel-tools
104-
unless (memq (gptel-get-tool name) gptel-tools)
105-
do (throw 'mismatch t))
106-
(or (equal (sort val #'string-lessp) ;preset tools same as gptel-tools
107-
(sort (mapcar #'gptel-tool-name gptel-tools)
108-
#'string-lessp))
109-
(throw 'mismatch t))))
104+
(setq val (cl-loop ; Check against tool names, not tools (faster with sorting)
105+
for tool in (ensure-list (gptel--modify-value gptel-tools val))
106+
for tool-name = (or (and (stringp tool) tool)
107+
(ignore-errors (gptel-tool-name tool)))
108+
if (not (member tool-name uniq-tool-names))
109+
collect tool-name into uniq-tool-names
110+
finally return uniq-tool-names))
111+
(or (equal (sort val #'string-lessp) ;preset tools same as gptel-tools?
112+
(sort (mapcar #'gptel-tool-name gptel-tools)
113+
#'string-lessp))
114+
(throw 'mismatch t)))
110115
(t (let* ((suffix (substring
111116
(if (symbolp key) (symbol-name key) key) 1))
112117
(sym (or (intern-soft (concat "gptel-" suffix))
113118
(intern-soft (concat "gptel--" suffix)))))
119+
;; FIXME(modify-list): Fix for values specified with a spec, like :eval
114120
(or (null sym)
115121
(and (boundp sym) (equal (eval sym) val))
116122
(throw 'mismatch t)))))))))
@@ -530,6 +536,7 @@ which see."
530536
('t "buffer-locally")
531537
(_ "globally")))
532538
gptel--known-presets nil t)))))
539+
(gptel--set-with-scope 'gptel--preset name gptel--set-buffer-locally)
533540
(gptel--apply-preset
534541
name (lambda (sym val)
535542
(gptel--set-with-scope sym val gptel--set-buffer-locally)))

gptel.el

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,7 @@ form it is returned as is.
312312
- :function will call val with ORIGINAL as its argument, and return the result.
313313
- :merge will treat ORIGINAL and NEW-SPEC as plists and return a merged plist,
314314
with NEW-SPEC taking precedence."
315-
(if (not (and (consp new-spec)
316-
(symbolp (car new-spec))
317-
(string-prefix-p ":" (symbol-name (car new-spec)))))
315+
(if (not (and (consp new-spec) (keywordp (car new-spec))))
318316
new-spec
319317
(let ((current original) (tail new-spec))
320318
(while tail

0 commit comments

Comments
 (0)