s-kostyaev / ellama

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

Wrong diff sent to LLM for commit message for partial commit #175

Open neic opened 5 days ago

neic commented 5 days ago

When using ellama-generate-commit-message, I am always getting a generated commit message for the diff for the unstaged changes even though I am committing a partial commit. Since #146 I would expect only the staged changes to be in the diff.

I am not a lips hacker, but I fumbled around a bit with the debugger. It showed that the (vc-git-diff-switches "--cached") binding is not picked up by the vc-git-internal call:

Debugger entered--returning value: ("--histogram")
  #<subr vc-switches>(git diff)
* apply(#<subr vc-switches> (git diff))
* vc-switches(git diff)
  vc-git-diff(nil nil nil #<buffer  *temp*> nil)
  vc-call-backend(Git diff nil nil nil #<buffer  *temp*> nil)
  vc-diff-internal(nil (Git nil) nil nil nil #<buffer  *temp*>)
  ellama--diff-cached()
  ellama-generate-commit-message()
  funcall-interactively(ellama-generate-commit-message)
  command-execute(ellama-generate-commit-message)

Found by running M-x debug-on-entry vc-switches and stepping through with d.

I fumbled around a bit more and got it to work by moving the binding around the with-temp-buffer:

(defun ellama--diff-cached ()
  "Diff staged."
  (let* ((default-directory
      (if (string= ".git"
               (car (reverse
                 (cl-remove
                  ""
                  (file-name-split default-directory)
                  :test #'string=))))
          (file-name-parent-directory default-directory)
        default-directory))
     (diff (let ((vc-git-diff-switches "--cached"))
                 (with-temp-buffer
           (vc-diff-internal
            nil (vc-deduce-fileset t) nil nil nil (current-buffer))
           (buffer-substring-no-properties (point-min) (point-max))))))
    (if (string-empty-p diff)
    nil
      diff)))

I do not know why the let works and the let* does not. (let* ((a 2) (b 3) (c a)) (+ b c)) does eval to 5.

I can whip up a PR if this solution is the way to fix it.

s-kostyaev commented 5 days ago

Hi @neic. Thank you for report and debugging. Please provide a PR. Have you tested your solution?

s-kostyaev commented 3 days ago

@neic I can't reproduce your issue. Which emacs version do you have?