Fix gptel-menu's transient-setup errors by shuffling Instructions/Context titles

[huonw]

This fixes #348 by moving how the empty line between the system prompt description and the Instructions/Context columns is configured:

• previously: empty "" lines before both individual columns, which seems caused issues with the use of :pad-keys t, with errors like transient-setup: Wrong type argument: (or eieio-object cl-structure-object oclosure), "" as reported in gptel-menu gives error #348
• now: append the empty line to the end of the description, and have the columns just have their title configured.

This also swaps the Context title and :pad-keys t, which seems to be required.

NB. retaining both "" and "Context" and just moving :pad-keys t after it also doesn't resolve the problem:

   [""
    "Context"
    :pad-keys t

Debugger entered--Lisp error: (excessive-lisp-nesting 1601)
  transient--parse-child(gptel-menu :pad-keys)
  transient--parse-child(gptel-menu :pad-keys)
  transient--parse-child(gptel-menu :pad-keys)
  transient--parse-child(gptel-menu :pad-keys)
  #f(compiled-function (s) #<bytecode -0x143445264900cf8e>)(:pad-keys)
  cl-mapcan(#f(compiled-function (s) #<bytecode -0x143445264900cf8e>) ("Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)))
  transient--parse-group(gptel-menu ["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)])
  transient--parse-child(gptel-menu ["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)])
  #f(compiled-function (s) #<bytecode -0x143445264900cf8e>)(["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)])
  cl-mapcan(#f(compiled-function (s) #<bytecode -0x143445264900cf8e>) (["Instructions" ("s" "Set system message" gptel-system-prompt :transient t) (gptel--infix-add-directive)] ["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)]))
  transient--parse-group(gptel-menu [:description (lambda nil (concat (string-replace "\n" "⮐ " (truncate-string-to-width gptel--system-message (max (- ... 12) 14) nil nil t)) "\n")) ["Instructions" ("s" "Set system message" gptel-system-prompt :transient t) (gptel--infix-add-directive)] ["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)]])
  transient--parse-child(gptel-menu [:description (lambda nil (concat (string-replace "\n" "⮐ " (truncate-string-to-width gptel--system-message (max (- ... 12) 14) nil nil t)) "\n")) ["Instructions" ("s" "Set system message" gptel-system-prompt :transient t) (gptel--infix-add-directive)] ["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)]])
  #f(compiled-function (s) #<bytecode -0x143445264900cf8e>)([:description (lambda nil (concat (string-replace "\n" "⮐ " (truncate-string-to-width gptel--system-message (max (- ... 12) 14) nil nil t)) "\n")) ["Instructions" ("s" "Set system message" gptel-system-prompt :transient t) (gptel--infix-add-directive)] ["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)]])
  cl-mapcan(#f(compiled-function (s) #<bytecode -0x143445264900cf8e>) ([:description (lambda nil (concat (string-replace "\n" "⮐ " (truncate-string-to-width gptel--system-message (max ... 14) nil nil t)) "\n")) ["Instructions" ("s" "Set system message" gptel-system-prompt :transient t) (gptel--infix-add-directive)] ["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)]] [["Request Parameters" :pad-keys t (gptel--infix-variable-scope) (gptel--infix-provider) (gptel--infix-max-tokens) (gptel--infix-num-messages-to-send :if (lambda nil (or gptel-mode gptel-track-response))) (gptel--infix-temperature :if (lambda nil gptel-expert-commands)) (gptel--infix-use-context) (gptel--infix-track-response :if (lambda nil (and gptel-expert-commands (not gptel-mode))))] ["Prompt from" ("m" "Minibuffer instead" "m") ("y" "Kill-ring instead" "y") "" ("i" "Respond in place" "i")] ["Response to" ("e" "Echo area instead" "e") ("g" "gptel session" "g" :class transient-option :prompt "Existing or new gptel session: " :reader (lambda (prompt _ _history) (read-buffer prompt (generate-new-buffer-name ...) nil (lambda ... ... ...)))) ("b" "Any buffer" "b" :class transient-option :prompt "Output to buffer: " :reader (lambda (prompt _ _history) (read-buffer prompt (buffer-name ...) nil))) ("k" "Kill-ring" "k")]] [["Send" (gptel--suffix-send) ("M-RET" "Regenerate" gptel--regenerate :if gptel--in-response-p)] [:description gptel--refactor-or-rewrite :if use-region-p ("r" (lambda nil (if (derived-mode-p ...) "Refactor" "Rewrite")) gptel-rewrite-menu)] ["Tweak Response" :if gptel--in-response-p :pad-keys t ("SPC" "Mark" gptel--mark-response) ("P" "Previous variant" gptel--previous-variant :if gptel--at-response-history-p :transient t) ("N" "Next variant" gptel--previous-variant :if gptel--at-response-history-p :transient t) ("E" "Ediff previous" gptel--ediff :if gptel--at-response-history-p)] ["Dry Run" :if (lambda nil (or gptel-log-level gptel-expert-commands)) ("I" "Inspect query (Lisp)" (lambda nil "Inspect the query that will be sent as a lisp obje..." (interactive) (gptel--sanitize-model) (gptel--inspect-query (gptel--suffix-send ...)))) ("J" "Inspect query (JSON)" (lambda nil "Inspect the query that will be sent as a JSON obje..." (interactive) (gptel--sanitize-model) (gptel--inspect-query (gptel--suffix-send ...) 'json)))]]))
  #f(compiled-function (arg1 arg2 &rest rest) "Define NAME as a transient prefix command.\n\nARGLIST are the arguments that command takes.\nDOCSTRING is the documentation string and is optional.\n\nThese arguments can optionally be followed by key-value pairs.\nEach key has to be a keyword symbol, either `:class' or a keyword\nargument supported by the constructor of that class.  The\n`transient-prefix' class is used if the class is not specified\nexplicitly.\n\nGROUPs add key bindings for infix and suffix commands and specify\nhow these bindings are presented in the popup buffer.  At least\none GROUP has to be specified.  See info node `(transient)Binding\nSuffix and Infix Commands'.\n\nThe BODY is optional.  If it is omitted, then ARGLIST is also\nignored and the function definition becomes:\n\n  (lambda ()\n    (interactive)\n    (transient-setup \\='NAME))\n\nIf BODY is specified, then it must begin with an `interactive'\nform that matches ARGLIST, and it must call `transient-setup'.\nIt may however call that function only when some condition is\nsatisfied; that is one of the reason why you might want to use\nan explicit BODY.\n\nAll transients have a (possibly nil) value, which is exported\nwhen suffix commands are called, so that they can consume that\nvalue.  For some transients it might be necessary to have a sort\nof secondary value, called a scope.  Such a scope would usually\nbe set in the commands `interactive' form and has to be passed\nto the setup function:\n\n  (transient-setup \\='NAME nil nil :scope SCOPE)" #<bytecode 0x1dd4039275b10756>)(gptel-menu nil "Change parameters of prompt to send to the LLM." [:description (lambda nil (concat (string-replace "\n" "⮐ " (truncate-string-to-width gptel--system-message (max (- ... 12) 14) nil nil t)) "\n")) ["Instructions" ("s" "Set system message" gptel-system-prompt :transient t) (gptel--infix-add-directive)] ["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)]] [["Request Parameters" :pad-keys t (gptel--infix-variable-scope) (gptel--infix-provider) (gptel--infix-max-tokens) (gptel--infix-num-messages-to-send :if (lambda nil (or gptel-mode gptel-track-response))) (gptel--infix-temperature :if (lambda nil gptel-expert-commands)) (gptel--infix-use-context) (gptel--infix-track-response :if (lambda nil (and gptel-expert-commands (not gptel-mode))))] ["Prompt from" ("m" "Minibuffer instead" "m") ("y" "Kill-ring instead" "y") "" ("i" "Respond in place" "i")] ["Response to" ("e" "Echo area instead" "e") ("g" "gptel session" "g" :class transient-option :prompt "Existing or new gptel session: " :reader (lambda (prompt _ _history) (read-buffer prompt (generate-new-buffer-name (concat "*" ... "*")) nil (lambda (buf-name) (if ... ...) (let ... ...))))) ("b" "Any buffer" "b" :class transient-option :prompt "Output to buffer: " :reader (lambda (prompt _ _history) (read-buffer prompt (buffer-name (other-buffer)) nil))) ("k" "Kill-ring" "k")]] [["Send" (gptel--suffix-send) ("M-RET" "Regenerate" gptel--regenerate :if gptel--in-response-p)] [:description gptel--refactor-or-rewrite :if use-region-p ("r" (lambda nil (if (derived-mode-p 'prog-mode) "Refactor" "Rewrite")) gptel-rewrite-menu)] ["Tweak Response" :if gptel--in-response-p :pad-keys t ("SPC" "Mark" gptel--mark-response) ("P" "Previous variant" gptel--previous-variant :if gptel--at-response-history-p :transient t) ("N" "Next variant" gptel--previous-variant :if gptel--at-response-history-p :transient t) ("E" "Ediff previous" gptel--ediff :if gptel--at-response-history-p)] ["Dry Run" :if (lambda nil (or gptel-log-level gptel-expert-commands)) ("I" "Inspect query (Lisp)" (lambda nil "Inspect the query that will be sent as a lisp obje..." (interactive) (gptel--sanitize-model) (gptel--inspect-query (gptel--suffix-send (cons "I" ...))))) ("J" "Inspect query (JSON)" (lambda nil "Inspect the query that will be sent as a JSON obje..." (interactive) (gptel--sanitize-model) (gptel--inspect-query (gptel--suffix-send (cons "I" ...)) 'json)))]] (interactive) (gptel--sanitize-model) (transient-setup 'gptel-menu))
  macroexpand((transient-define-prefix gptel-menu nil "Change parameters of prompt to send to the LLM." [:description (lambda nil (concat (string-replace "\n" "⮐ " (truncate-string-to-width gptel--system-message (max ... 14) nil nil t)) "\n")) ["Instructions" ("s" "Set system message" gptel-system-prompt :transient t) (gptel--infix-add-directive)] ["" "Context" :pad-keys t (gptel--infix-context-add-region) (gptel--infix-context-add-buffer) (gptel--infix-context-add-file) (gptel--suffix-context-buffer)]] [["Request Parameters" :pad-keys t (gptel--infix-variable-scope) (gptel--infix-provider) (gptel--infix-max-tokens) (gptel--infix-num-messages-to-send :if (lambda nil (or gptel-mode gptel-track-response))) (gptel--infix-temperature :if (lambda nil gptel-expert-commands)) (gptel--infix-use-context) (gptel--infix-track-response :if (lambda nil (and gptel-expert-commands (not gptel-mode))))] ["Prompt from" ("m" "Minibuffer instead" "m") ("y" "Kill-ring instead" "y") "" ("i" "Respond in place" "i")] ["Response to" ("e" "Echo area instead" "e") ("g" "gptel session" "g" :class transient-option :prompt "Existing or new gptel session: " :reader (lambda (prompt _ _history) (read-buffer prompt (generate-new-buffer-name ...) nil (lambda ... ... ...)))) ("b" "Any buffer" "b" :class transient-option :prompt "Output to buffer: " :reader (lambda (prompt _ _history) (read-buffer prompt (buffer-name ...) nil))) ("k" "Kill-ring" "k")]] [["Send" (gptel--suffix-send) ("M-RET" "Regenerate" gptel--regenerate :if gptel--in-response-p)] [:description gptel--refactor-or-rewrite :if use-region-p ("r" (lambda nil (if (derived-mode-p ...) "Refactor" "Rewrite")) gptel-rewrite-menu)] ["Tweak Response" :if gptel--in-response-p :pad-keys t ("SPC" "Mark" gptel--mark-response) ("P" "Previous variant" gptel--previous-variant :if gptel--at-response-history-p :transient t) ("N" "Next variant" gptel--previous-variant :if gptel--at-response-history-p :transient t) ("E" "Ediff previous" gptel--ediff :if gptel--at-response-history-p)] ["Dry Run" :if (lambda nil (or gptel-log-level gptel-expert-commands)) ("I" "Inspect query (Lisp)" (lambda nil "Inspect the query that will be sent as a lisp obje..." (interactive) (gptel--sanitize-model) (gptel--inspect-query (gptel--suffix-send ...)))) ("J" "Inspect query (JSON)" (lambda nil "Inspect the query that will be sent as a JSON obje..." (interactive) (gptel--sanitize-model) (gptel--inspect-query (gptel--suffix-send ...) 'json)))]] (interactive) (gptel--sanitize-model) (transient-setup 'gptel-menu)))
  elisp--eval-defun()
  #<subr eval-defun>(nil)
  edebug--eval-defun(#<subr eval-defun> nil)
  apply(edebug--eval-defun #<subr eval-defun> nil)
  eval-defun(nil)
  funcall-interactively(eval-defun nil)
  command-execute(eval-defun)

[0x715C]

I tried this patch on the 0.9.0 release with transient 0.7.4 and it works for me.

guix time-machine -- shell --with-patch=emacs-gptel=$(guix download https://patch-diff.githubusercontent.com/raw/karthink/gptel/pull/361.patch | sed 'N;s/\n.*//') --pure emacs emacs-gptel emacs-transient -- emacs

[karthink]

Thanks for the PR. I'd prefer to fix the issue from Transient's side instead of working around it by modifying a different menu element. This will cause unexpected issues when changing the menu layout in the future.

[huonw]

Ah okay. I see where you're coming from.

I was assuming the intent was "leave some space after the long description text" in which case I think this PR expresses that intent better (by putting the space once in the description, rather than on every one of the immediately-following columns), but if the intent is something else, then of course that's irrelevant.

[karthink]

Ah okay. I see where you're coming from. I was assuming the intent was "leave some space after the long description text" in which case I think this PR expresses that intent better (by putting the space once in the description, rather than on every one of the immediately-following columns), but if the intent is something else, then of course that's irrelevant.

I'm looking for a minimal reproducer that I can ask Tarsius about -- better to fix this bug in transient. (And if it's not a bug, better to document that :pad-keys and empty strings can't be used together.) I posted an example of a minimal menu that should reproduce this bug, but I can't reproduce the bug in gptel-menu either so I'm not sure yet.

[karthink]

Is this bug still present on the latest version of Transient?