karthink / gptel

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

Enhance mode name handling in directives #346

Open nfedyashev opened 1 month ago

nfedyashev commented 1 month ago

This would fix the existing issue: You are an enh-ruby programmer. Refactor the following code.

This is a bit similar to existing gptel--strip-mode-suffix(used in gptel--rewrite-message) but for.. prefixes as well?

Related links on enh-ruby major mode: https://github.com/zenspider/enhanced-ruby-mode https://stackoverflow.com/questions/30318515/what-are-advantages-of-enh-ruby-mode-vs-ruby-mode

karthink commented 1 month ago

Well there's no easy heuristic way to solve this. I can provide an option where you can populate major-mode -> mode string associations (e.g. ((enh-ruby-mode . "ruby"))) for use by gptel, but perhaps there's a way to do it automatically by consulting one of the several major-mode-related settings in Emacs? auto-mode-alist, major-mode-remap-alist etc.

nfedyashev commented 1 month ago

Unfortunately, I'm not familiar with the modules that you mentioned so I'm not sure if there is a better way

From what I experienced, it is not a major issue in this particular case(at least not for gpt4) and LLMs can resolve this prompt ambiguity

nfedyashev commented 1 month ago

Related stackoverflow topic

nfedyashev commented 2 weeks ago

same issue with js2 Improved JavaScript editing mode for Emacs

I don't mind too much but I think for a typical user it might be rather annoying to edit this prompt every time. Takes about 4 seconds(depends on keyboard settings) just to get to that chunk of text using backspace/arrows.

nfedyashev commented 2 weeks ago

As a workaround, I'm using this patched version of the related function.

(defun gptel--strip-mode-suffix (mode-sym)
  "Remove the -mode suffix from MODE-NAME.

MODE-NAME is typically a major-mode symbol."
  (cond
   ((eq mode-sym 'enh-ruby-mode) "ruby")
   ((eq mode-sym 'js2-mode) "js")
   (t
    (let ((mode-name (thread-last
                       (symbol-name mode-sym)
                       (string-remove-suffix "-mode")
                       (string-remove-suffix "-ts"))))
      (if (provided-mode-derived-p
           mode-sym 'prog-mode 'text-mode 'tex-mode)
          mode-name "")))))
karthink commented 6 days ago

N.B.: I just added gptel-rewrite-directives-hook, to which you can add functions to get specialized refactor instructions based on context. I'm still debating adding a gptel-mode-replacement-alist with some common transformations like js2 to javascript.

The rewrite interface has also been redesigned from scratch, if you use it please let me know what you think. (it may not be apparent right away but trying a refactoring will make it clear)

nfedyashev commented 2 days ago

@karthink

The rewrite interface has also been redesigned from scratch, if you use it please let me know what you think. (it may not be apparent right away but trying a refactoring will make it clear)

I tried it and it looks great :+1: It is pretty self-explanatory, I could use on the first try.

Unfortunately, I had to switch back to my gptel--strip-mode-suffix fork.

I think even a quick and dirty hotfix that fixes it for 70-80% users is better than the one where an average gptel user would need to figure out how to write custom hooks(or perhaps some examples might be handy in README ).

karthink commented 2 days ago

I think even a quick and dirty hotfix that fixes it for 70-80% users is better than the one where an average gptel user would need to figure out how to write custom hooks(or perhaps some examples might be handy in README ).

Yes, I plan to include hooks for some common cases. Besides the git commit message buffer, do you have any others in mind?

EDIT: Excuse me, I replied to the wrong thread.

karthink commented 2 days ago

For this topic, yes, I plan to add a gptel-mode-name-remap-alist populated with some common replacements so this should be fixed soon.

nfedyashev commented 2 days ago

@karthink

FYI Perhaps the comment below from Dmitry Gutov(author of some other Emacs packages) on this topic might be helpful

And your question about a better way: unfortunately we don't have an unequivocal major-mode=>language mapping anywhere yet, but there is another source of information: major-mode-remap-alist and (most recently) major-mode-remap-defaults. So one could also check whether the current mode is associated as a value with some major mode as a key which fits the traditional naming scheme (when the mapped one doesn't).

But I would only bother with it after finding a real-world mode that doesn't fit the current detector. That mapping is only used for TeX/LaTeX and ts modes by default.

Originally from this gist thread

karthink commented 2 days ago

Yes, see my first comment above. These variables (like major-mode-remap-alist) are very bare, it doesn't help.