karthink / gptel

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

Add posframe render for `ChatGPT`'s transient memu #110

Closed eval-exec closed 8 months ago

eval-exec commented 9 months ago

I add a posframe handler for gptel's transient menu. It's my first attempt to write emacs lisp. Do you think this is a good idea?

image

image

karthink commented 8 months ago

@eval-exec This is a neat idea, thanks for the PR. That said, I'm trying to avoid piling on dependencies in gptel, and would like to avoid depending on posframe.

After testing it for a bit, the posframe display also works better for actions with immediate feedback (like completions, help-at-point etc). For async tasks with a delay of a few seconds (like with gptel), popping up the frame at (point) is somewhat jarring. This can be addressed by making the posframe appear at a fixed location in the frame.

It's my first attempt to write emacs lisp.

It's really neat for a first attempt! A few notes:

  1. gptel-override-parameters is a free variable, it needs to be defined with defvar in the file. Better yet, you can create a local (scoped) variable in a let block around it. Byte-compiling the file usually highlights such issues.
  2. You don't need to specify the else clause of an if expression as nil -- it's nil by default. It's cleaner to use when instead of if in such cases.
  3. Assigning gptel-posframe-border-color to a fixed color makes it not work with either light or dark themes. It might be better to define a face.
  4. define-keymap is an Emacs 29 feature. gptel supports Emacs 27.1, so you can't use it directly. Also, your use of define-keymap returns a keymap that isn't used anywhere -- this is probably not what you want.