Closed markokocic closed 1 month ago
Thank you! It was planned long time ago, but I can't find free time to implement it
The only thing I'm not sure is the naming. I followed the names and menu structure from the table of all shortcuts in README, but I'm not sure if that's the best hierarchy overall. I can make a PR on Monday and then we can refine it.
Ok, I created the PR.
Before merging, I think it would make sense to align key bindings and menu structure, so that people end up executing the same command by typing the same keys, regardless if they define ellama prefix to "C-c e" or bind "C-e e" to the ellama-transient-main-menu
.
I personally bind "C-c c" to ellama-chat
as the most frequently used command, and "C-c e" to the menu.
maybe we can use better hierarchy. Do you have any ideas?
I tried to define a new structure for the menu. I regrouped commands a bit.
The issue is, submenus need different keys, so I ended up with s
for Summarize and S
for Session. Also, I'm not sure about "Make", should it be grouped or be in its own menu.
I don't write transient menus yet. What's the difference between grouping and separated menus?
I'm also not an expert in transient, was just playing with it to create a menu for ellama to make it more discoverable for me.
You can define one of menu groups in the menu to put together related commands or submenus together. LIke for example in this example I put "System" group which groups "Session", "Context" and "Provider" submenus. You can define group names as you wish, but each submenu shortcut needs to be unique inside a menu.
In this case, those shortcuts just open another submenu, but it would also be possible to have commands assigned to them.
Let it be submenu then. I think it makes sense.
All menus and submenus in PR are defined with transient-define-prefix
. You can try it out to see what makes the most sense for you.
I gave it a try and I like it a lot, makes it way easier to use. One thing I noticed, is that there is a bit of inconsistency when it comes to the keybindings. However they are coming from ellama itself and not this specific pull request. It just highlights them more.
E.g. C-c e a i
says Chat, while maybe "Interactive Chat" would make more sense.
Then "Text" has four sub menus while all the actions for code are hidden behind "c".
Inside "Text" we have the following two functions but I'm not sure they make sense withing the "Text" context since the function names suggest that they are more related to the interactive chat.
I just had a quick look into it so I might be wrong about these findings.
("e" "Enable Translation" ellama-chat-translation-enable)
("d" "Disable Translation" ellama-chat-translation-disable)
I based menu system based on keybindings, without knowing what every entry mean. Feel free to reorganise it.
C-c e a i
was "ask interactively" before. I think it should be c
at the top level, and code actions should be under C
. Probably we can improve it more.
Translations of chat session should not messing up with text actions. I'm not sure where to put it.
I updated the menus in PR. Chat moved to top level as "c" Code shortcut changed to "C" Provider selection moved to top level, since it's the only command in own menu.
I have added a couple comments. But in general LGTM. Address comments and hopefully we will merge it.
Instead of having to memorize a bunch of keybindings for different options, it would be nice if ellama would come out of the box with a menu bound to key binding. I created one for me, using
transient
package, and bind in to aC-c e
prefix in my config file.I created this menu system and grouped commands to groups based on prefixed. You may review the menu organization and include it with the base package.
Once menus are defined, you can bind them to your use-package config like this: