Skip to content

fix(ediff): restore chat side-window after ediff session #41

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

Merged
ericdallo merged 10 commits into from
Sep 12, 2025
Merged

fix(ediff): restore chat side-window after ediff session #41

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
86304a1
fix(ediff): restore chat side-window after ediff session
ChetanKoneru Sep 12, 2025
2ea0ffa
enhance ediff function
ChetanKoneru Sep 12, 2025
3dddc5b
fix formatting
ChetanKoneru Sep 12, 2025
6f4abbe
feat(ediff): add Doom Emacs compatibility for window and popup handling
ChetanKoneru Sep 12, 2025
8fecd3f
decouple eco-chat.el , new file eca-diff.el
ChetanKoneru Sep 12, 2025
bec3555
clean up eca-chat.el
ChetanKoneru Sep 12, 2025
4f27339
remove duplicates and clean up
ChetanKoneru Sep 12, 2025
3cc84f5
clean up eco-chat.el
ChetanKoneru Sep 12, 2025
f19a046
package `f` is already present
ChetanKoneru Sep 12, 2025
36dd0b9
remove ediff and smerge requirement from eca-caht
ChetanKoneru Sep 12, 2025
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
256 changes: 33 additions & 223 deletions eca-chat.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
(require 'eca-util)
(require 'eca-api)
(require 'eca-mcp)
(require 'eca-diff)
Comment thread
ChetanKoneru marked this conversation as resolved.

(require 'evil nil t)

Expand Down Expand Up @@ -145,8 +146,7 @@ Must be a valid model supported by server, check `eca-chat-select-model`."
(defcustom eca-chat-diff-tool 'ediff
"Select the method for displaying file-change diffs in ECA chat."
:type '(choice (const :tag "Side-by-side Ediff" ediff)
(const :tag "Merge-style Smerge" smerge)
(const :tag "Plain text diff" text))
(const :tag "Merge-style Smerge" smerge))
:group 'eca)


Expand Down Expand Up @@ -922,227 +922,6 @@ If FORCE? decide to OPEN? or not."
(propertize (concat "-" (number-to-string (plist-get details :linesRemoved))) 'font-lock-face 'error)
" "))

(defun eca-chat--parse-unified-diff (diff-text)
"Parse DIFF-TEXT and return a plist with :original and :new strings."
(let ((orig '()) (new '()) in-hunk)
(dolist (l (split-string diff-text "\n"))
(cond
((string-match "^@@.*@@$" l) (setq in-hunk t))
((and in-hunk (string-prefix-p " " l))
(push (substring l 1) orig) (push (substring l 1) new))
((and in-hunk (string-prefix-p "-" l))
(push (substring l 1) orig))
((and in-hunk (string-prefix-p "+" l))
(push (substring l 1) new))))
(list :original (string-join (nreverse orig) "\n")
:new (string-join (nreverse new) "\n"))))


(defun eca-chat--show-diff-text (path diff)
"Show DIFF for file at PATH as plain unified diff text.
Adds \\='q\\=' to quit."
(with-current-buffer (get-buffer-create (format "*eca-diff:%s*" path))
(let ((inhibit-read-only t))
(erase-buffer)
(insert diff)
(diff-mode)
(goto-char (point-min))
;; Add a local 'q' binding to kill this buffer quickly
(let ((map (make-sparse-keymap)))
(set-keymap-parent map (current-local-map))
(define-key map (kbd "q") (lambda () (interactive) (kill-buffer (current-buffer))))
(use-local-map map)))
(pop-to-buffer (current-buffer))))

(defun eca-chat--show-diff-ediff (path diff)
"Show DIFF for file at PATH using Ediff side-by-side in windows.
Uses window configuration management instead of creating frames.

If the current window is a side window, temporarily clear side-window
protections so Ediff can split windows freely. The original window
configuration is restored when Ediff quits via the cleanup hook."
(let* ((parsed (eca-chat--parse-unified-diff diff))
(orig (plist-get parsed :original))
(new (plist-get parsed :new))
(buf-orig (generate-new-buffer (format "*eca-diff-orig:%s*" path)))
(buf-new (generate-new-buffer (format "*eca-diff-new:%s*" path)))
(cwc (current-window-configuration))
(orig-selected (selected-window))
(ediff-buffers-before
(seq-filter (lambda (b)
(string-match-p "\\*\\(ediff-\\|Ediff Control\\)" (buffer-name b)))
(buffer-list)))
(session-ediff-buffers nil)
(cleanup-fn nil)
(after-setup-fn nil))
;; Fill temporary buffers
(with-current-buffer buf-orig
(let ((inhibit-read-only t))
(erase-buffer)
(insert orig)
(set-buffer-modified-p nil)))
(with-current-buffer buf-new
(let ((inhibit-read-only t))
(erase-buffer)
(insert new)
(set-buffer-modified-p nil)))

;; Temporarily relax side-window protections in the current frame so
;; Ediff can split windows. The original configuration will be
;; restored by `cleanup-fn' (which calls `set-window-configuration').
(let ((frame (selected-frame)))
(dolist (w (seq-filter (lambda (w)
(and (eq (window-frame w) frame)
(or (window-parameter w 'no-delete-other-windows)
(window-parameter w 'window-side))))
(window-list)))
(when (window-live-p w)
(set-window-parameter w 'no-delete-other-windows nil)
(set-window-parameter w 'window-side nil))))

;; Ensure Ediff has a single full window to manage (it will split it).
(unless (one-window-p t)
(delete-other-windows))

;; Cleanup: restore windows and kill temp/session buffers
(setq cleanup-fn
(lambda ()
;; Restore window configuration saved at the beginning
(when (window-configuration-p cwc)
(set-window-configuration cwc))
;; Ensure focus returns to the original window if still live
(when (window-live-p orig-selected)
(select-window orig-selected))
(when (buffer-live-p buf-orig) (kill-buffer buf-orig))
(when (buffer-live-p buf-new) (kill-buffer buf-new))
(dolist (b session-ediff-buffers)
(when (and b (buffer-live-p b))
(kill-buffer b)))
(remove-hook 'ediff-quit-hook cleanup-fn)))

;; After-setup hook: capture ediff buffers and move to first diff
(setq after-setup-fn
(lambda ()
(let ((ediff-buffers-after
(seq-filter (lambda (b)
(string-match-p "\\*\\(ediff-\\|Ediff Control\\)" (buffer-name b)))
(buffer-list))))
(setq session-ediff-buffers
(seq-filter (lambda (b)
(and (not (member b ediff-buffers-before))
(not (string-match-p "\*Ediff Registry\*" (buffer-name b)))))
ediff-buffers-after)))
(condition-case _err
(progn
(setq ediff-current-difference -1)
(ediff-next-difference))
(error nil))
(remove-hook 'ediff-after-setup-windows-hook after-setup-fn)))

(add-hook 'ediff-quit-hook cleanup-fn)
(add-hook 'ediff-after-setup-windows-hook after-setup-fn)

;; Start Ediff in the prepared window environment. Ediff will manage
;; splits from here on. On error, restore original windows/config.
(condition-case err
(ediff-buffers buf-orig buf-new)
(error
;; On error remove hooks and kill temps, and restore windows
(remove-hook 'ediff-quit-hook cleanup-fn)
(remove-hook 'ediff-after-setup-windows-hook after-setup-fn)
(when (window-configuration-p cwc)
(set-window-configuration cwc))
(when (buffer-live-p buf-orig) (kill-buffer buf-orig))
(when (buffer-live-p buf-new) (kill-buffer buf-new))
(message "eca-chat: error starting ediff: %s" err)))))

(defun eca-chat--show-diff-smerge (path diff)
"Show DIFF for file at PATH using Smerge in a dedicated window.
Uses regular window management instead of creating frames.

If the current window is a side window, temporarily clear side-window
protections so `delete-other-windows' can succeed. The original window
configuration is restored when the smerge buffer is killed."
(let* ((parsed (eca-chat--parse-unified-diff diff))
(orig (plist-get parsed :original))
(new (plist-get parsed :new))
(buf (generate-new-buffer (format "*eca-smerge:%s*" path)))
(cwc (current-window-configuration))
(orig-selected (selected-window))
(frame (selected-frame))
(cleanup-running nil)
cleanup-fn
window-config-hook)

;; Fill buffer with conflict markers
(with-current-buffer buf
(let ((inhibit-read-only t))
(erase-buffer)
(insert (concat "<<<<<<< Original\n" orig "\n=======\n" new "\n>>>>>>> New\n"))
(diff-mode)
(smerge-mode 1)
(goto-char (point-min))))

;; Define cleanup that restores window configuration and kills the buffer
(setq cleanup-fn
(lambda ()
(unless cleanup-running
(setq cleanup-running t)
(when (window-configuration-p cwc)
(ignore-errors (set-window-configuration cwc)))
(when (window-live-p orig-selected)
(select-window orig-selected))
(when (buffer-live-p buf)
(with-current-buffer buf
(remove-hook 'kill-buffer-hook cleanup-fn t)))
(when (functionp window-config-hook)
(remove-hook 'window-configuration-change-hook window-config-hook))
(when (buffer-live-p buf)
(kill-buffer buf)))))

;; If buffer is no longer visible, run cleanup
(setq window-config-hook
(lambda ()
(unless (get-buffer-window buf t)
(funcall cleanup-fn))))
(add-hook 'window-configuration-change-hook window-config-hook)

;; Add local keymap and kill hook to the smerge buffer
(with-current-buffer buf
(let ((map (make-sparse-keymap)))
(set-keymap-parent map (current-local-map))
(define-key map (kbd "q") (lambda () (interactive) (funcall cleanup-fn)))
(use-local-map map))
(add-hook 'kill-buffer-hook cleanup-fn nil t))

;; Temporarily relax side-window protections in the current frame so
;; `delete-other-windows' can succeed. The original window
;; configuration is restored by `cleanup-fn' (which calls `set-window-configuration').
(dolist (w (seq-filter (lambda (w)
(and (eq (window-frame w) frame)
(or (window-parameter w 'no-delete-other-windows)
(window-parameter w 'window-side))))
(window-list)))
(when (window-live-p w)
(set-window-parameter w 'no-delete-other-windows nil)
(set-window-parameter w 'window-side nil)))
;; Present the buffer full-frame
(when (window-live-p (frame-root-window frame))
(select-window (frame-root-window frame)))
(unless (one-window-p t)
(delete-other-windows))
(switch-to-buffer buf)

;; Return nil explicitly
nil))

(defun eca-chat--show-diff (path diff)
"Dispatch DIFF view based on `eca-chat-diff-tool` for PATH."
(pcase eca-chat-diff-tool
('ediff (eca-chat--show-diff-ediff path diff))
('smerge (eca-chat--show-diff-smerge path diff))
(_ (eca-chat--show-diff-text path diff))))

(defun eca-chat--relativize-filename-for-workspace-root (filename roots)
"Relativize the FILENAME if a workspace root is found for ROOTS."
(or (-some->> (-first (lambda (root) (f-ancestor-of? root filename)) roots)
Expand Down Expand Up @@ -1318,6 +1097,37 @@ of (LINE . CHARACTER) representing the current selection or cursor position."
(declare-function evil-delete-backward-word "evil" ())
(declare-function evil-delete-back-to-indentation "evil" ())

(defun eca-chat--parse-unified-diff (diff-text)
"Compatibility wrapper that delegates to `eca-diff-parse-unified-diff'.

DIFF-TEXT is the unified diff string to parse and returns the parsed
plist produced by `eca-diff-parse-unified-diff'."
(eca-diff-parse-unified-diff diff-text))

(defun eca-chat--show-diff-ediff (path diff)
"Compatibility wrapper delegating to `eca-diff-show-ediff'.

PATH is the file path being shown and DIFF is the unified diff text.
This wrapper passes the current buffer as CHAT-BUF so `eca-diff' can
restore the chat display after Ediff quits."
(eca-diff-show-ediff path diff (current-buffer) (lambda (b) (ignore-errors (eca-chat--display-buffer b)))))


(defun eca-chat--show-diff-smerge (path diff)
"Compatibility wrapper delegating to `eca-diff-show-smerge'.

PATH is the file path being shown and DIFF is the unified diff text.
This wrapper passes the current buffer as CHAT-BUF so `eca-diff' can
restore the chat display after smerge quits."
(eca-diff-show-smerge path diff (current-buffer) (lambda (b) (ignore-errors (eca-chat--display-buffer b)))))
Comment on lines +1098 to +1120

@ericdallo ericdallo Sep 12, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need these 3 functions? can't we just do what they do inside the pcase below?

@ChetanKoneru ChetanKoneru Sep 12, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just want to keep it clean and maintainable.

@ericdallo ericdallo Sep 12, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see low level and more indirection having like that, but no big deal too, I'm ok keeping it



(defun eca-chat--show-diff (path diff)
"Dispatch DIFF view based on `eca-chat-diff-tool` for PATH."
(pcase eca-chat-diff-tool
('ediff (eca-chat--show-diff-ediff path diff))
('smerge (eca-chat--show-diff-smerge path diff))))

;; Public

(define-derived-mode eca-chat-mode markdown-mode "eca-chat"
Expand Down
Loading
Loading