karthink / gptel

A simple LLM client for Emacs
GNU General Public License v3.0
1.04k stars 113 forks source link

prompts dialog #140

Closed vzaliva closed 6 months ago

vzaliva commented 7 months ago

I have several prompts set up. When I chose "select directive for chat," there was no way to exit from this dialogue. Choosing a directive via the associated shortcut key sets it but leaves it open on the screen. I have to kill it with C-g. I am using helm, if that matters.

karthink commented 7 months ago

the associated shortcut key sets it but leaves it open on the screen

Could you share a screenshot of this? I can't reproduce it and am having trouble picturing what you mean.

vzaliva commented 7 months ago

image

karthink commented 7 months ago

image

If I understand correctly: when you press a key in this screen (say p for Paper), the directive is updated, but this screen stays open. You then have to press C-g to return to the previous menu?

vzaliva commented 7 months ago

that's correct

karthink commented 7 months ago

Have you updated gptel in a while? This is very old behavior that was changed in #96.

vzaliva commented 7 months ago

This is MELPA version I have:

 Status: Installed in ‘gptel-20231113.211/’ (unsigned). Delete
 Version: 20231113.211
 Commit: 17a58d38e7299f254d02c29bbcc9211146394758
karthink commented 7 months ago

Hmm, I'm not sure what could be causing it then :thinking:. Selecting a directive takes me back to the main gptel-menu .

vzaliva commented 7 months ago

here is what I see in Messages buffer:

Directive: You are my proofreader. I write in british english. The posts is for general audience (like forums and social media) and the tone needs to be brief and not overly formal. Proofreead and brush up whatever I send to you and summarize the changes you made. and: Args out of range: #s(transient-prefix #s(transient-prefix eieio--unbound gptel-system-prompt eieio--unbound nil eieio--unbound eieio--unbound eieio--unbound nil nil ...) gptel-system-prompt 4 nil eieio--unbound nil eieio--unbound nil (nil) ...), 22 [2 times] and: Args out of range: #s(transient-prefix #s(transient-prefix eieio--unbound gptel-menu eieio--unbound nil eieio--unbound eieio--unbound eieio--unbound nil nil ...) gptel-menu 4 nil eieio--unbound nil eieio--unbound nil (nil) ...), 22

vzaliva commented 7 months ago

I treid toggle-debug-on-error and here is more detail:

Debugger entered--Lisp error: (args-out-of-range #<transient-prefix transient-prefix-158ba39d45a0> 22)
  eieio-oref(#<transient-prefix transient-prefix-158ba39d45a0> unwind-suffix)
  (and t (eieio-oref prefix 'unwind-suffix))
  (let* ((unwind (and t (eieio-oref prefix 'unwind-suffix)))) (if unwind (progn (transient--debug 'unwind-command) (funcall unwind suffix)) nil))
  (unwind-protect (apply fn args) (let* ((unwind (and t (eieio-oref prefix 'unwind-suffix)))) (if unwind (progn (transient--debug 'unwind-command) (funcall unwind suffix)) nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil))
  (closure ((advice-interactive closure ((advice lambda (fn &rest args) (interactive #4) (apply '#0 fn args)) (suffix . gptel-system-prompt) (prefix . #<transient-prefix transient-prefix-158ba39d45a0>) pp-default-function Man-notify-method t) (spec) (let ((abort t)) (unwind-protect (prog1 (advice-eval-interactive-spec spec) (setq abort nil)) (if abort (progn ... ... ...))))) (advice lambda (fn &rest args) (interactive (closure #3 (spec) (let (...) (unwind-protect ... ...)))) (apply '#0 fn args)) (suffix . gptel-system-prompt) (prefix . #<transient-prefix transient-prefix-158ba39d45a0>) pp-default-function Man-notify-method t) (fn &rest args) (unwind-protect (apply fn args) (let* ((unwind (and t (eieio-oref prefix ...)))) (if unwind (progn (transient--debug 'unwind-command) (funcall unwind suffix)) nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil)))(#<subr gptel-system-prompt>)
  apply((closure ((advice-interactive closure ((advice lambda (fn &rest args) (interactive #5) (apply '#1 fn args)) (suffix . gptel-system-prompt) (prefix . #<transient-prefix transient-prefix-158ba39d45a0>) pp-default-function Man-notify-method t) (spec) (let ((abort t)) (unwind-protect (prog1 (advice-eval-interactive-spec spec) (setq abort nil)) (if abort (progn ... ... ...))))) (advice lambda (fn &rest args) (interactive (closure #4 (spec) (let (...) (unwind-protect ... ...)))) (apply '#1 fn args)) (suffix . gptel-system-prompt) (prefix . #<transient-prefix transient-prefix-158ba39d45a0>) pp-default-function Man-notify-method t) (fn &rest args) (unwind-protect (apply fn args) (let* ((unwind (and t (eieio-oref prefix ...)))) (if unwind (progn (transient--debug 'unwind-command) (funcall unwind suffix)) nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil))) #<subr gptel-system-prompt> nil)
  (lambda (fn &rest args) (interactive (closure ((advice . #0) (suffix . gptel-system-prompt) (prefix . #<transient-prefix transient-prefix-158ba39d45a0>) pp-default-function Man-notify-method t) (spec) (let ((abort t)) (unwind-protect (prog1 (advice-eval-interactive-spec spec) (setq abort nil)) (if abort (progn ... ... ...)))))) (apply '(closure ((advice-interactive closure (... ... ... pp-default-function Man-notify-method t) (spec) (let ... ...)) (advice . #0) (suffix . gptel-system-prompt) (prefix . #<transient-prefix transient-prefix-158ba39d45a0>) pp-default-function Man-notify-method t) (fn &rest args) (unwind-protect (apply fn args) (let* (...) (if unwind ... nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil))) fn args))(#<subr gptel-system-prompt>)
  apply((lambda (fn &rest args) (interactive (closure ((advice . #1) (suffix . gptel-system-prompt) (prefix . #<transient-prefix transient-prefix-158ba39d45a0>) pp-default-function Man-notify-method t) (spec) (let ((abort t)) (unwind-protect (prog1 (advice-eval-interactive-spec spec) (setq abort nil)) (if abort (progn ... ... ...)))))) (apply '(closure ((advice-interactive closure (... ... ... pp-default-function Man-notify-method t) (spec) (let ... ...)) (advice . #1) (suffix . gptel-system-prompt) (prefix . #<transient-prefix transient-prefix-158ba39d45a0>) pp-default-function Man-notify-method t) (fn &rest args) (unwind-protect (apply fn args) (let* (...) (if unwind ... nil)) (advice-remove suffix advice) (eieio-oset prefix 'unwind-suffix nil))) fn args)) #<subr gptel-system-prompt> nil)
  gptel-system-prompt()
  funcall-interactively(gptel-system-prompt)
  command-execute(gptel-system-prompt)
sergeyplis commented 7 months ago

I have a very similar problem and update to the latest 20231120 did not fix it. An additional problem is inability to edit the prompt - emacs is not changing focus to the prompt buffer and there is no way to do it (C+x+b and mouse clock do not work)

vzaliva commented 7 months ago

I am using gptel a lot and am very interested in getting this bug fixed. Please let me know if I can help diagnose it. I know some Lisp and can try debugging things if you tell me what to do exactly, since I am not very familiar with emacs development.

karthink commented 7 months ago

@vzaliva what is your transient version? M-x describe-package transient

vzaliva commented 7 months ago
     Status: Installed in ‘transient-20231127.1512/’,
             shadowing a built-in package (unsigned).
    Version: 20231127.1512
     Commit: 600b2df42535ca7077252f37dedd111349b76116
    Summary: Transient commands
   Requires: emacs-26.1, compat-29.1.4.1, seq-2.24
Required by: magit-20231120.2359, gptel-20231120.1925, git-commit-20231030.2243
    Website: https://github.com/magit/transient
   Keywords: extensions 
 Maintainer: Jonas Bernoulli <jonas@bernoul.li>
     Author: Jonas Bernoulli <jonas@bernoul.li>
Other versions: 20231127.1512 (melpa), 0.4.3 (gnu), builtin.
karthink commented 7 months ago

@vzaliva Thank you. I see that you're running the latest transient version - we can check if this bug is caused by this as follows:

  1. Run emacs -q
  2. Add MELPA to the package archives,
    (require 'package)
    (add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/") t)
    (package-initialize)
  3. M-x package-install gptel.

This should use Emacs' built-in transient version. Please let me know if the bug occurs here.

vzaliva commented 7 months ago

I tried to do this. package-install does not find gptel however it is shown as already installed (via MELPA). The bug is still there even with emacs -q

vzaliva commented 7 months ago

P.S. I also tried emacs -Q

karthink commented 7 months ago

Try moving or remaining your package cache (typically ~/.emacs.d/elpa) first, or perhaps

emacs -q --init-directory=/tmp/test

On Tue, Nov 28, 2023, 9:32 PM Vadim Zaliva @.***> wrote:

I tried to do this. package-install does not find gptel however it is shown as already installed (via MELPA). The bug is still there even with emacs -q

— Reply to this email directly, view it on GitHub https://github.com/karthink/gptel/issues/140#issuecomment-1831249765, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBVOLEYVXODGTZT5SXUK73YG3CFJAVCNFSM6AAAAAA7TJC3UOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZRGI2DSNZWGU . You are receiving this because you commented.Message ID: @.***>

vzaliva commented 7 months ago

I've renamed my .emacs and .emac.d and did fresh install of gptel. The result. is the same. Error messages from Messages buffer:

Directive: You are a large language model and a careful programmer. Provide code and only code as output without any additional text, prompt or note.
Unbound suffix: ‘RET’ (Use ‘C-g’ to abort, ‘?’ for help) [newline]
Error in pre-command-hook (transient--pre-command): (wrong-type-argument stringp #[0 "  \300!\207" [#<buffer *Messages*> menu-bar-select-buffer-function] 2 nil nil])
Mark set

Also some Warnings:

Warning (comp): seq-25.el:445:16: Warning: ‘seq-contains’ is an obsolete generic function (as of 27.1); use ‘seq-contains-p’ instead. Disable showing Disable logging
Warning (comp): gptel-openai.el:102:53: Warning: the function ‘prop-match-end’ is not known to be defined. Disable showing Disable logging
Warning (comp): gptel-openai.el:101:53: Warning: the function ‘prop-match-beginning’ is not known to be defined. Disable showing Disable logging

However I see that when I've installed gptel, a new version of transient was installed:

~/.emacs.d $ fdfind transient
eln-cache/28.2-521777fc/gptel-transient-964c700e-71c966b8.eln
eln-cache/28.2-521777fc/transient-7580a622-caf0e686.eln
elpa/gptel-20231120.1925/gptel-transient.el
elpa/gptel-20231120.1925/gptel-transient.elc
elpa/transient-20231128.1536/
elpa/transient-20231128.1536/transient-autoloads.el
elpa/transient-20231128.1536/transient-pkg.el
elpa/transient-20231128.1536/transient.el
elpa/transient-20231128.1536/transient.elc
elpa/transient-20231128.1536/transient.info

The version:

     Status: Installed in ‘transient-20231128.1536/’,
             shadowing a built-in package (unsigned).
    Version: 20231128.1536
     Commit: 9050a0d058041e5e54d4e16359fd061e779c2ecb
    Summary: Transient commands
   Requires: emacs-26.1, compat-29.1.4.4, seq-2.24
Required by: gptel-20231120.1925
    Website: https://github.com/magit/transient
   Keywords: extensions 
 Maintainer: Jonas Bernoulli <jonas@bernoul.li>
     Author: Jonas Bernoulli <jonas@bernoul.li>
Other versions: 20231128.1536 (melpa), 0.5.0 (gnu), builtin.
vzaliva commented 7 months ago

I suspect it is because gptel MELPA package explicitly requires transient-0.4.0 and this version does not match builtin transient so it upgrades it. Here is M-x describe-package transient before MELPA:

Package transient is built-in.

     Status: Built-In.
    Summary: Transient commands
karthink commented 7 months ago

My bad, I thought Emacs 29 shipped with transient version > 0.4.

On Tue, Nov 28, 2023, 9:55 PM Vadim Zaliva @.***> wrote:

I suspect it is because gptel MELPA package explicitly requires transient-0.4.0 and this version does not match builtin transient so it upgrades it. Here is M-x describe-package transient before MELPA:

Package transient is built-in.

 Status: Built-In.
Summary: Transient commands

— Reply to this email directly, view it on GitHub https://github.com/karthink/gptel/issues/140#issuecomment-1831266659, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBVOLE3HMOJ3KVTMZJLV7TYG3E5VAVCNFSM6AAAAAA7TJC3UOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZRGI3DMNRVHE . You are receiving this because you commented.Message ID: @.***>

karthink commented 7 months ago

I see you are on Emacs 28 -- this is why the built-in version of Transient does not suffice. Let me try to reproduce it with the latest build of Transient.

karthink commented 7 months ago

@vzaliva It looks like recent changes deep inside the Transient library broke gptel -- I have no idea how to fix this. A couple of other transient menus I use (not gptel) are broken too.

vzaliva commented 7 months ago

FYI, Transient just updated (through MELPA) to ‘transient-20231129.1714/’ but the bug is still there.

Perhaps transient changed the API? I can try to dig further if you can get me something to play with. Can you give me a minimal transient use example which works with the old one but does not work with the new transient? I can try to make it work and report back what changes are required.

karthink commented 7 months ago

Can you give me a minimal transient use example which works with the old one but does not work with the new transient?

I can't, unfortunately. Here is a minimal version of the "select a directive" menu:


(transient-define-prefix demo-transient ()
  [:description "test"
   :class transient-column
   :setup-children demo-transient--setup])

(defun demo-transient--setup (_)
  (transient-parse-suffixes
   'demo-transient
   `((demo-transient-c)
     ("a" "choose a" (lambda () (interactive) (message "a")))
   ("b" "choose b" (lambda () (interactive) (message "b"))))))

(transient-define-suffix demo-transient-c ()
  "Choose option c"
  :description "choose c"
  :key "c"
  (interactive)
  (message "c"))

However choosing any suffix here exits the transient as expected. I'm not sure what's causing the problem with gptel's version. If you want to take a look, the problem is contained to the definitions of gptel-system-prompt, gptel-system-prompt--setup and gptel--suffix-system-message.

vzaliva commented 7 months ago

There was an update of transient package, and something changed. The new version is 20231204.33. When I select a directive, it no longer gives me an error message. Even after setting toggle-debug-on-error all I have now is the following in the Messages buffer:

Debug on Error enabled globally
Directive: You are a large language model living in Emacs
                and a helpful assistant. Respond concisely.

It also shows in the transient buffer which does not close (as before):

image

holgerschurig commented 6 months ago

If I understand correctly: when you press a key in this screen (say p for Paper), the directive is updated, but this screen stays open. You then have to press C-g to return to the previous menu?

That happened for me, too. BTW, I'm using gptel ...

holger@desktop:~/.doom.d/repos/gptel$ git rev-parse HEAD
8d3e08faa88cefb89f8e90e99cd2b2513b8f04b9

Emacs / transient is 29.1.99 which is GIT ...

holger@desktop:~/.doom.d/emacs.git$ git rev-parse HEAD
2922d683b7899b8b580cbff466478617ea7ad5ad
holger@desktop:~/.doom.d/emacs.git$ git branch
* emacs-29
  master
karthink commented 6 months ago

@holgerschurig Thanks. I'm going to ask tarsius (the author of the Transient menu system) for help with this when I get some time, since the behavior differs between Transient versions.

vzaliva commented 6 months ago

For people like me who could no longer use gptel: I switched to https://github.com/xenodium/chatgpt-shell and am quite happy with it.

karthink commented 6 months ago

@holgerschurig I think it should be fixed now, could you update and try again?

holgerschurig commented 6 months ago

@holgerschurig I think it should be fixed now, could you update and try again?

Yes, I can confirm that it is now fixed for me!

holgerschurig commented 6 months ago

For people like me who could no longer use gptel: I switched to https://github.com/xenodium/chatgpt-shell and am quite happy with it.

Well, I'm using gptel not with ChatGPT or any other online service, but with local Ollama. First, I don't want to depend on some external entity that I don't trust and e.g. transfer parts of my (or my employers) source code to it. And I also can never be sure what they do with my data, what they collect --- as most of these companies are in the USA, which never embraced privacy like the EU.

So thanks for the tip, but thanks no :-)

karthink commented 6 months ago

Yes, I can confirm that it is now fixed for me!

Good to know, I'll close this issue now.

Well, I'm using gptel not with ChatGPT or any other online service, but with local Ollama.

Just a heads-up -- resuming saved chats (from saved files) with Ollama is not fully supported yet, there will be issues where the model doesn't remember the full conversation. I plan to work on fixing this.

And I also can never be sure what they do with my data, what they collect --- as most of these companies are in the USA, which never embraced privacy like the EU.

And free/local models appear to be improving quite fast. I'm curious about what will be possible using only a home computer in a couple of years.

kofm commented 6 months ago

Hello @karthink,

I'm sorry I don't have more detailed information, but I'm still experiencing the bug described in this issue, even after updating both gptel and transient to the latest commits from their main branches. Specifically, after pressing h twice to edit the prompt, the cursor remains stuck in the menu, necessitating a manual C-g command to exit. Post-exit, I can edit the prompt buffer and confirm with C-c, and everything functions normally.

Additionally, a similar issue occurs when selecting a crowdsourced prompt: after choosing one, the text loads into the prompt buffer for editing, but the focus remains on the menu. Again, I have to manually kill the transient menu to shift focus to the prompt buffer, edit the prompt, and save it with C-c to return to the main menu of gptel-menu.

Please let me know if there's any more data I can provide to assist in diagnosing this issue.

Best regards.

karthink commented 6 months ago

@vzaliva FWIW between this and #157 all bugs with the transient menu should be fixed now.