xenodium / chatgpt-shell

A multi-llm Emacs shell (ChatGPT, Claude, Gemini, Ollama, Perplexity) + editing integrations
https://lmno.lol/alvaro
GNU General Public License v3.0
866 stars 77 forks source link

[Feature Request] Support multiple shell instances #122

Closed djliden closed 1 year ago

djliden commented 1 year ago

It would be useful if, as with e.g. eshell, one could use C-u chatgpt-shell to open an additional chatgpt-shell instance in order to have multiple ongoing chatgpt sessions.

@xenodium mentioned in #84 that this would take some work to add. I'm happy to contribute and work on this issue.

xenodium commented 1 year ago

Thanks for filing the feature request.

Would be good to flesh out the behaviour we'd like to implement, so we can make changes based on the spec'ed goal.

An initial area that comes to mind are all the commands that send prompts to the shell from other buffers. Gut feeling is to ensure there's always one *chatgpt* buffer available to receive these.

Also, functions like chatgpt-shell-swap-system-prompt and chatgpt-shell-swap-model-version should prolly work on buffer-local values.

Buffer names also come to mind. While numbering like *chatgpt* *chatgpt*<2> *chatgpt*<3> are a good start, we may want to tackle allowing renaming also, so it's easier to swap between, say "chatgpt programming" and "chatgpt Spanish tutor".

@xenodium mentioned in #84 that this would take some work to add. I'm happy to contribute and work on this issue.

That would be great. I may have some patches somewhere to get this started, but not sure if still revelant or apply cleanly.

xenodium commented 1 year ago

I may have some patches somewhere to get this started, but not sure if still revelant or apply cleanly.

I've added some patches in this branch: https://github.com/xenodium/chatgpt-shell/tree/multi-session

Creating new instances can be done via C-u chatgpt-shell. Not had much of a chance to fully test it. Please have a play with it and see if you run into issues.

ps. @gopar you may be keen also, as per #84.

djliden commented 1 year ago

Thank you! I will try it out.

djliden commented 1 year ago

I pulled down the new branch and played around with the new functionality a bit—adding a new shell (or shells) works great & I think is already a huge value add. I haven't gone into a ton of detail, but I tried:

An initial area that comes to mind are all the commands that send prompts to the shell from other buffers. Gut feeling is to ensure there's always one chatgpt buffer available to receive these.

This would be a heavier lift, but ultimately it would be useful to be able to either (1) specify an "active" shell at any given time, which will be the default target for prompts sent from other buffers; and/or (2) be able to associate certain buffers with certain shells -- e.g. state, maybe with file-local variables or a set-buffer-chatgpt-shell function, which chatgpt shell should be the target from which buffer; and/or (3) be able to interactively select the target chatgpt shell with each invocation of a prompt send command.

Thanks again for adding the multi-session functionality—great feature to have!

xenodium commented 1 year ago

Thanks for testing things out.

(1) specify an "active" shell at any given time, which will be the default target for prompts sent from other buffers;

I've made some changes to partially go in this direction.

There are some other changes like automatic buffer renaming (based on version and system prompt).

Pull the latest and give it another spin if you can. Not had much time to give it thorough testing. May have some bugs.

xenodium commented 1 year ago

@djliden @gopar I've fixed a handful of multi-session issues and merged the branch to main. Please let me know if you run into issues. The current implementation should hopefully satisfy the original feature request.

djliden commented 1 year ago

I am running into one issue, which might be at least partially configuration-dependent. I now get the following upon starting chatgpt-shell and when trying to set a primary shell:

shell-maker-start: Invalid function: shell-maker--with-temp-buffer-if.

Moving the new macros to the top of the file solves the issue. I think this is related to byte-compilation:

In order for compilation of macro calls to work, the macros must already be defined in Lisp when the calls to them are compiled. The compiler has a special feature to help you do this: if a file being compiled contains a defmacro form, the macro is defined temporarily for the rest of the compilation of that file.

https://www.gnu.org/software/emacs/manual/html_node/elisp/Compiling-Macros.html

When I test with emacs -Q and just load the package with load-file shell-maker.el and load-file chatgpt-shell.el, I don't encounter this issue, likely because, in that case, I'm not loading the byte-compiled file.

Anyway—I can dig a little further and make a PR moving those macros once I get a better sense of what's happening.

EDIT: See #124

xenodium commented 1 year ago

I'm OK with moving the macros for now if that solves things for ya. Thanks for the change!