s-kostyaev / ellama

Ellama is a tool for interacting with large language models from Emacs.
GNU General Public License v3.0
378 stars 27 forks source link

feat/custom_keymap_and_line_breaking #7

Closed LionyxML closed 7 months ago

LionyxML commented 9 months ago

Hello there! Thanks for this awesome package!

I was customizing it a bit and making a blog post about it when I decided to perform a couple of additions, I hope this surprise PR is OK with you :)

Demo:

https://github.com/s-kostyaev/ellama/assets/16169950/4dc6c39e-29e9-432b-a44c-7ad02623c650

1. Adds a custom keymap (default C-x e ...) to the main commands of ellama. This is customizable via 2 variables, on to turn the feature ON/OFF, another to change "C-x e" to another prefix.

C-x e ? will list available commands

M-x ellama will show defined keybindings

C-x e (wait) will show available commands if which key mode is on

C-x e c - will chat, C-x e a will ask about selection, and so on:

image

As you see I only added a few major features, if you wish, complementing after will be easy.

2. Adds breaking lines to all ellama* buffers by adding the visual-line mode to the buffer. Customizable via another custom variable.

Note: I wanted all changes to be available as soon as set/reseted, the only way I could find to do so was defining this defcustom after the functions were created, so the :set lambda function could call it.

This is nice because instead of this:

image

We get this:

image

These are the new entries to the customize-group ellama:

image
s-kostyaev commented 9 months ago

Nice work, thank you. But we should make CI happy. Please fix issues from CI and we will continue review.

s-kostyaev commented 9 months ago

Also, I think you can add more commands to keymap.

LionyxML commented 9 months ago

Great! I didn't see a CI before, my bad.

I ran checkdock and accepted it all (including some double spaces after . rule that changed another part of the code, I think there's` nothing bad doing so). Also package-lint does not produce any more errors or warnings.

I added the missing functions and tried to make the keybindings closer to the meaning of the description, but some were random since I could not find any good letters, lol.

s-kostyaev commented 9 months ago

@LionyxML you could skip double spaces warning, but it's fine that you have fixed it.

About random characters: maybe we need to group some commands, like "c" for code, then "cc" will be code change and "ce" will be code enhance? It will make chords longer but easy to remember.

LionyxML commented 9 months ago

Well, i thinkered quite a bit with some ideas, I ended up following the functions names (since those will be shown on C-x e ? or C-x (waiting, if which key is present), I think typing something close to the function name will help the user to remember what he want's.

image

Any suggestions? I am running out of ideas, lol.

s-kostyaev commented 8 months ago

My idea:

(let ((key-commands
       '(;; code
     ("c c" ellama-complete-code "Code complete")
     ("c a" ellama-add-code "Code add")
     ("c e" ellama-change-code "Code edit")
     ("c i" ellama-enhance-code "Code improve")
     ("c r" ellama-code-review "Code review")
     ;; summarize
     ("s s" ellama-summarize "Summarize")
     ("s w" ellama-summarize-webpage "Summarize webpage")
     ;; improve
     ("i w" ellama-enhance-wording "Improve wording")
     ("i g" ellama-enhance-grammar-spelling "Improve grammar and spelling")
     ("i c" ellama-make-concise "Improve conciseness")
     ;; make
     ("m l" ellama-make-list "Make list")
     ("m t" ellama-make-table "Make table")
     ("m f" ellama-render "Make format")
     ;; ask
     ("a a" ellama-ask-about "Ask about")
     ("a c" ellama-ask "Chat with ellama") ;; not sure
     ;; other
     ("t"     ellama-translate "Translate the selected region")
     ("d"   ellama-define-word "Define selected word")))))

We can also create command aliases to match this keys if it will be more concise. @LionyxML What do you think?

LionyxML commented 8 months ago

Nice! Just commited the suggested changes and added the aliases.

About ellama-ask, I think it could be something like ask interactive, ask buffer or something alike. On code I shipped the commit with:

("a i" ellama-ask-interactive "Chat with ellama")

But we can change it.

Also added some more info on the README.

s-kostyaev commented 8 months ago

@LionyxML have you signed the FSF copyright assignment (CA)? I have a plan to move this project to GNU Elpa and all significant contributors should do it for that. Since this PR contains more than 15 loc, it matters. Sorry, that this not in Readme yet, I have offered for this yesterday and like the idea.

LionyxML commented 8 months ago

Thank you for bringing this to my attention, and I'm glad to hear that you're planning to move the project to GNU Elpa. I have no problems signing the FSF copyright assignment to support this effort.

Could you please provide me with instructions on how to proceed with the signing process? I'd be happy to follow the necessary steps to ensure that my contributions are in line with the project's goals.

s-kostyaev commented 8 months ago

Send an email to emacs-devel@gnu.org to request the form and follow instructions.

LionyxML commented 8 months ago

Just sent the form request. I'll keep you in touch of its progress.

LionyxML commented 8 months ago

Btw, as a follow up, I just sent the signed document to GNU.

s-kostyaev commented 8 months ago

@LionyxML I have added ellama-ask-selection and ellama-ask-line commands. You can rebase and add it to keymap.

LionyxML commented 8 months ago

Great! I'll work on that tomorrow.

s-kostyaev commented 7 months ago

@LionyxML rebase on latest main and fix some minor things and lets merge it finally

s-kostyaev commented 7 months ago

@LionyxML also after some rework maybe line breaking part not needed now

LionyxML commented 7 months ago

@s-kostyaev Just rebased, made the changes and got rid of the breaking line mode (I tested without it and it is now ok).

LionyxML commented 7 months ago

CI seems unhappy with signatures: https://github.com/s-kostyaev/ellama/actions/runs/7241271145/job/19725194852?pr=7#step:4:16

LionyxML commented 7 months ago

Silly push restarted the CI, it looks ok now.

s-kostyaev commented 7 months ago

@LionyxML Thank you for contribution!

s-kostyaev commented 7 months ago

Default key prefix changed to C-c e to prevent overwriting default emacs binding kmacro-end-and-call-macro