Skip to content

feat: use magit-diff if magit is installed #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
phye wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: use magit-diff if magit is installed #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 28 additions & 22 deletions inline-review.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,10 @@ posted a new comment)."
"Pop up a read-only buffer with `git diff --stat' between source and target branch.
Fetches the latest refs from origin each time and diffs against remote-tracking
branches (origin/source vs origin/target) to ensure the stat reflects the most
up-to-date remote state rather than potentially stale local branches."
up-to-date remote state rather than potentially stale local branches.

If `magit' is installed, uses `magit-diff' instead so you can jump to
files directly from the diff buffer."
(interactive)
(unless (inline-review--review-in-progress-p)
(user-error
Expand Down Expand Up @@ -386,28 +389,31 @@ up-to-date remote state rather than potentially stale local branches."
;; Fetch the two branches from origin to ensure we have up-to-date refs.
(call-process "git" nil nil nil
"fetch" "origin" source target)
(let ((outbuf (get-buffer-create "*inline-review-overview*")))
(with-current-buffer outbuf
(let ((inhibit-read-only t))
(erase-buffer)
(let ((rc (call-process "git" nil (list outbuf t) nil
"diff" "--stat"
remote-target remote-source)))
(if (and (integerp rc) (zerop rc))
(progn
(if (fboundp 'magit-diff-range)
(magit-diff-range (concat remote-target "..." remote-source)

@phye phye Jun 1, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not work

@phye phye Jun 1, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

magit-diff with --stat will not work

'("--stat"))
(let* ((outbuf (get-buffer-create "*inline-review-overview*")))
(with-current-buffer outbuf
(let ((inhibit-read-only t))
(erase-buffer)
(let ((rc (call-process "git" nil (list outbuf t) nil
"diff" "--stat"
remote-target remote-source)))

@phye phye Jun 1, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks fine

(if (and (integerp rc) (zerop rc))
(progn
(goto-char (point-min))
(view-mode 1))
(let ((err (string-trim (buffer-string))))
(erase-buffer)
(insert
(format "git diff --stat %s %s failed%s\n"
remote-target remote-source
(if (string-empty-p err)
""
(format ": %s" err))))
(goto-char (point-min))
(view-mode 1))
(let ((err (string-trim (buffer-string))))
(erase-buffer)
(insert
(format "git diff --stat %s %s failed%s\n"
remote-target remote-source
(if (string-empty-p err)
""
(format ": %s" err))))
(goto-char (point-min))
(view-mode 1))))))
(pop-to-buffer outbuf)))))
(view-mode 1))))))
(pop-to-buffer outbuf))))))

;;;###autoload
(defun inline-review-set-backend-for-repo (backend)
Expand Down