Skip to content

[Feature Request] Add transient menu (prototype included) inspired by Aidermacs/Magit #341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rogsme opened this issue Apr 1, 2025 · 14 comments · May be fixed by #343
Open

[Feature Request] Add transient menu (prototype included) inspired by Aidermacs/Magit #341

rogsme opened this issue Apr 1, 2025 · 14 comments · May be fixed by #343

Comments

@rogsme
Copy link

rogsme commented Apr 1, 2025

Hey!

This isn’t really an issue; more of a suggestion and a small contribution from my side.

I often found myself wanting a quick menu to remind me of the things I can do, and I thought chatgpt-shell could benefit from something similar to Aidermacs's transient menu:

Image

So I went ahead and created a transient menu for chatgpt-shell. Here’s a short video showing how it works:

demo.mp4

Let me know if this is something you'd like in the package! I’d be happy to open a PR in the next few days! 👍🏾

@xenodium
Copy link
Owner

xenodium commented Apr 4, 2025

Ooh yay! Thanks for the proposal. Sorry for the delay. I've been meaning to add transient for quite some time. I had a rough patch somewhere, but was for the compose buffer.

I've been contemplating either getting rid of some of these commands I see listed as they can be easily replaced by chatgpt-shell-quick-insert or maybe re-implemt using the same insertion mechanism.

One thing I'm wondering is how to deal with vanilla vs evil bindings. I'm not super familiar with evil, but j/k are vim bindings right? Do you know how other packages do it for their transient menus?

@rogsme
Copy link
Author

rogsme commented Apr 4, 2025

Ooh yay! Thanks for the proposal. Sorry for the delay. I've been meaning to add transient for quite some time. I had a rough patch somewhere, but was for the compose buffer.

No worries my dude! I can take this one, no problem 😁

I've been contemplating either getting rid of some of these commands I see listed as they can be easily replaced by chatgpt-shell-quick-insert or maybe re-implemt using the same insertion mechanism.

If you have a list of commands you want added to the menu, I can replace them 👍🏾

One thing I'm wondering is how to deal with vanilla vs evil bindings. I'm not super familiar with evil, but j/k are vim bindings right?

I use evil bindings, and this menu works perfectly fine!

Do you know how other packages do it for their transient menus?

I don't know, but I can investigate! There might be a vanilla version we can use. I have some time to put into this this weekend, once I have an update I'll let you know 👍🏾

@rogsme
Copy link
Author

rogsme commented Apr 4, 2025

A quick update, we might be able to get away with using define-prefix-command. I'll try converting my menu to define-prefix-command and see if it works 👍🏾

@djr7C4
Copy link
Contributor

djr7C4 commented Apr 5, 2025

Nice! This looks like a good addition.

@rogsme rogsme linked a pull request Apr 6, 2025 that will close this issue
@xenodium
Copy link
Owner

I've been meaning to add transient for quite some time. I had a rough patch somewhere, but was for the compose buffer.

Ok, here's the compose buffer one. It shows up automatically after LLM provider replies, unless chatgpt-shell-compose-auto-transient is unset. Menu organization, bindings, etc. aren't final.

Image

@rogsme
Copy link
Author

rogsme commented Apr 23, 2025

That looks great! Sorry I haven’t been able to work on this lately, I’m on vacation overseas 😂

I’ll pick it back up next week when I’m back! But seriously, your menu looks way better than transient. Do you have a PR? Feel free to use mine as a base 👍🏾

@xenodium
Copy link
Owner

No worries at all. There's no hurry here.

I’m on vacation overseas 😂

Hope you're somewhere nice enjoying yourself!

But seriously, your menu looks way better than transient.

It's transient too :) 2aa82cd

@rogsme
Copy link
Author

rogsme commented Apr 23, 2025

Oh, it's already merged! I remember I went with a vertical menu instead of horizontal because transient doesn’t play well with split window buffers. That’s why magit and aider do the same thing too. I'll update chatgpt-shell and test! 🔥

And it's no worry haha, I'm on vacation but still working today 😂

@xenodium
Copy link
Owner

Oh, it's already merged!

This transient menu is for the compose buffer only. We still need your menu for the other buffers.

I went with a vertical menu instead of horizontal because transient doesn’t play well with split window buffers.

Good point. Let's see how well it works in practice. Happy to move to a more vertical approach if needed.

@xenodium
Copy link
Owner

I'm on vacation but still working today 😂

😭

@rogsme
Copy link
Author

rogsme commented Apr 24, 2025

Hey @xenodium! This is how it looks like in split mode:

Image

I guess vertical is the way to go then?

@xenodium
Copy link
Owner

I guess vertical is the way to go then?

Sounds like it. So a 2 column width max?

ps. The transient I submitted was mostly thinking in the context of compose buffers. Some of the functions are unlikely to work on the shell.

@rogsme
Copy link
Author

rogsme commented Apr 24, 2025

In my example, I was able to squeeze in a bit more short columns:

Image

Also, my PR worked across all of Emacs (not just in compose buffers) and it was interactive. When invoked with a region selected, the menu options would adapt accordingly. Would you like me to include that as well?

(I'm refering to the PR description in #343)

@xenodium
Copy link
Owner

Also, my PR worked across all of Emacs (not just in compose buffers) and it was interactive. When invoked with a region selected, the menu options would adapt accordingly. Would you like me to include that as well?

I think we need at least 2 transient menus. The compose one is very specific to compose buffer but also right after LLM replied and thus buffer goes read-only.

We can bring in your PR menu also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants