From 28770ca212eba3c66957b12e31d6afbdbfb3aab2 Mon Sep 17 00:00:00 2001 From: Daniel Colascione Date: Fri, 21 Mar 2025 19:46:08 -0400 Subject: [PATCH] Adapt ediff to nonstandard layouts Make ediff cope with having some of its windows (especially the control window) not shown by a custom ediff-window-setup-function. Modernize relevant adjacent code. After this change, one can write a custom ediff-window-setup-function that doesn't show the control window. * doc/misc/ediff.texi (Notes on Heavy-duty Customization): Refine language to explain that the window setup function doesn't have to show all windows. * lisp/vc/ediff-util.el (ediff-select-control-window-on-setup): New variable. (ediff-setup, ediff-recenter, ediff-recenter-one-window) (ediff-recenter-ancestor, ediff-toggle-read-only) (ediff-operate-on-windows, ediff-jump-to-difference-at-point) (ediff-default-suspend-function) (ediff-clone-buffer-for-region-comparison) (ediff-clone-buffer-for-window-comparison): Modernize control flow; select only windows that exist. * lisp/vc/ediff-wind.el (ediff-with-live-window): New convenience macro. (ediff-window-setup-function): Explain relaxed contract. (cherry picked from commit e5ee1d2a74c6a0989c863c3c6c06eba31efaecb3) --- doc/misc/ediff.texi | 6 +- lisp/vc/ediff-util.el | 229 +++++++++++++++++------------------------- lisp/vc/ediff-wind.el | 24 ++++- 3 files changed, 117 insertions(+), 142 deletions(-) diff --git a/doc/misc/ediff.texi b/doc/misc/ediff.texi index 26a0ee433d0..73f751f3a4b 100644 --- a/doc/misc/ediff.texi +++ b/doc/misc/ediff.texi @@ -2399,10 +2399,12 @@ The window displaying buffer A@. If buffer A is not visible, this variable is @code{nil} or it may be a dead window. @item ediff-window-B -The window displaying buffer B. +The window displaying buffer B. If buffer B is not visible, this variable +is @code{nil} or it may be a dead window. @item ediff-window-C -The window displaying buffer C, if any. +The window displaying buffer C, if any. If buffer C is not visible, +this variable is @code{nil} or it may be a dead window. @item ediff-control-frame A dedicated frame displaying the control buffer, if it exists. It is diff --git a/lisp/vc/ediff-util.el b/lisp/vc/ediff-util.el index 6064b581602..d80b79ee1c7 100644 --- a/lisp/vc/ediff-util.el +++ b/lisp/vc/ediff-util.el @@ -36,6 +36,15 @@ (require 'ediff-diff) (require 'ediff-merg) +(eval-when-compile + (require 'cl-lib)) + +(ediff-defvar-local ediff-select-control-window-on-setup t + "Select the control window after window setup. +`t' for compatibility. Custom `ediff-window-setup-function' +implementations may want to set it to `nil' to fully control +window setup.") + ;;; Functions @@ -472,20 +481,20 @@ to invocation.") (ediff-get-value-according-to-buffer-type 'C ediff-narrow-bounds)))) ;; position point in buf A - (save-excursion - (select-window ediff-window-A) - (goto-char shift-A)) + (when (window-live-p ediff-window-A) + (with-selected-window ediff-window-A + (goto-char shift-A))) ;; position point in buf B - (save-excursion - (select-window ediff-window-B) - (goto-char shift-B)) - (if ediff-3way-job - (save-excursion - (select-window ediff-window-C) - (goto-char shift-C))) - ) - - (select-window ediff-control-window) + (when (window-live-p ediff-window-B) + (with-selected-window ediff-window-B + (goto-char shift-B))) + (if (and ediff-3way-job (window-live-p ediff-window-C)) + (with-selected-window ediff-window-C + (goto-char shift-C)))) + + (when (and ediff-select-control-window-on-setup + (window-live-p ediff-control-window)) + (select-window ediff-control-window)) (ediff-visible-region) (mapc #'funcall startup-hooks) @@ -776,16 +785,19 @@ buffers." (or (not ediff-3way-job) (ediff-buffer-live-p ediff-buffer-C))) (progn - (or no-rehighlight + (or no-rehighlight (ediff-select-difference ediff-current-difference)) - (ediff-recenter-one-window 'A) - (ediff-recenter-one-window 'B) - (if ediff-3way-job - (ediff-recenter-one-window 'C)) + (save-current-buffer + (ediff-recenter-one-window 'A)) + (save-current-buffer + (ediff-recenter-one-window 'B)) + (if ediff-3way-job + (save-current-buffer + (ediff-recenter-one-window 'C))) - (ediff-with-current-buffer control-buf - (ediff-recenter-ancestor) ; check if ancestor is alive + (ediff-with-current-buffer control-buf + (ediff-recenter-ancestor) ; check if ancestor is alive (if (and (ediff-multiframe-setup-p) (not ediff-use-long-help-message) @@ -801,13 +813,11 @@ buffers." (ediff-with-current-buffer control-buf (ediff-refresh-mode-lines)) )) -;; this function returns to the window it was called from -;; (which was the control window) +;; this function does not change current window (defun ediff-recenter-one-window (buf-type) (if (ediff-valid-difference-p) ;; context must be saved before switching to windows A/B/C - (let* ((ctl-wind (selected-window)) - (shift (ediff-overlay-start + (let* ((shift (ediff-overlay-start (ediff-get-value-according-to-buffer-type buf-type ediff-narrow-bounds))) (job-name ediff-job-name) @@ -817,20 +827,16 @@ buffers." (window (if (window-live-p (symbol-value window-name)) (symbol-value window-name)))) - (if (and window ediff-windows-job) + (when (and window ediff-windows-job) (set-window-start window shift)) - (if window - (progn - (select-window window) - (deactivate-mark) - (ediff-position-region + (when window + (with-selected-window window + (deactivate-mark) + (ediff-position-region (ediff-get-diff-posn buf-type 'beg nil control-buf) (ediff-get-diff-posn buf-type 'end nil control-buf) (ediff-get-diff-posn buf-type 'beg nil control-buf) - job-name - ))) - (select-window ctl-wind) - ))) + job-name)))))) (defun ediff-recenter-ancestor () ;; do half-hearted job by recentering the ancestor buffer, if it is alive and @@ -838,21 +844,17 @@ buffers." (if (and (ediff-buffer-live-p ediff-ancestor-buffer) (ediff-valid-difference-p)) (let ((window (ediff-get-visible-buffer-window ediff-ancestor-buffer)) - (ctl-wind (selected-window)) (job-name ediff-job-name) (ctl-buf ediff-control-buffer)) (ediff-with-current-buffer ediff-ancestor-buffer (goto-char (ediff-get-diff-posn 'Ancestor 'beg nil ctl-buf)) - (if window - (progn - (select-window window) - (ediff-position-region + (when (window-live-p window) + (with-selected-window window + (ediff-position-region (ediff-get-diff-posn 'Ancestor 'beg nil ctl-buf) (ediff-get-diff-posn 'Ancestor 'end nil ctl-buf) (ediff-get-diff-posn 'Ancestor 'beg nil ctl-buf) - job-name)))) - (select-window ctl-wind) - ))) + job-name))))))) ;; This will have to be refined for 3way jobs @@ -1064,10 +1066,9 @@ of the current buffer." (sit-for 3)))) ; let the user see the warning (if (and toggle-ro-cmd (string-match "read-only-mode" (symbol-name toggle-ro-cmd))) - (save-excursion - (save-window-excursion - (select-window (ediff-get-visible-buffer-window buf)) - (command-execute toggle-ro-cmd))) + (save-window-excursion + (ediff-with-live-window (ediff-get-visible-buffer-window buf) + (command-execute toggle-ro-cmd))) (user-error "Don't know how to toggle read-only in buffer %S" buf)) ;; Check if we made the current buffer updatable, but its file is RO. @@ -1413,8 +1414,8 @@ Used in ediff-windows/regions only." (defun ediff-operate-on-windows (operation arg) ;; make sure windows aren't dead - (if (not (and (window-live-p ediff-window-A) (window-live-p ediff-window-B))) - (ediff-recenter 'no-rehighlight)) + (unless (and (window-live-p ediff-window-A) (window-live-p ediff-window-B)) + (ediff-recenter 'no-rehighlight)) (if (not (and (ediff-buffer-live-p ediff-buffer-A) (ediff-buffer-live-p ediff-buffer-B) (or (not ediff-3way-job) (ediff-buffer-live-p ediff-buffer-C)) @@ -1424,8 +1425,7 @@ Used in ediff-windows/regions only." )) (error ediff-KILLED-VITAL-BUFFER)) - (let* ((wind (selected-window)) - (wind-A ediff-window-A) + (let* ((wind-A ediff-window-A) (wind-B ediff-window-B) (wind-C ediff-window-C) (wind-Anc ediff-window-Ancestor) @@ -1438,26 +1438,16 @@ Used in ediff-windows/regions only." (coefAnc (if with-Ancestor (ediff-get-region-size-coefficient 'Ancestor operation)))) - (select-window wind-A) - (condition-case nil - (funcall operation (round (* coefA arg))) - (error)) - (select-window wind-B) - (condition-case nil - (funcall operation (round (* coefB arg))) - (error)) - (if three-way - (progn - (select-window wind-C) - (condition-case nil - (funcall operation (round (* coefC arg))) - (error)))) + (ediff-with-live-window wind-A + (ignore-errors (funcall operation (round (* coefA arg))))) + (ediff-with-live-window wind-B + (ignore-errors (funcall operation (round (* coefB arg))))) + (when three-way + (ediff-with-live-window wind-C + (ignore-errors (funcall operation (round (* coefC arg)))))) (when with-Ancestor - (select-window wind-Anc) - (condition-case nil - (funcall operation (round (* coefAnc arg))) - (error))) - (select-window wind))) + (ediff-with-live-window wind-Anc + (ignore-errors (funcall operation (round (* coefAnc arg)))))))) (defun ediff-scroll-vertically (&optional arg) "Vertically scroll buffers A, B (and C if appropriate). @@ -1817,44 +1807,29 @@ current point position in the specified buffer." (beg (if past-last-diff (ediff-with-current-buffer buffer (point-max)) (ediff-get-diff-posn buf-type 'beg (1- diff-no)))) - ctl-wind wind-A wind-B wind-C + wind-A wind-B wind-C shift) (if past-last-diff (ediff-jump-to-difference -1) (ediff-jump-to-difference diff-no)) - (setq ctl-wind (selected-window) - wind-A ediff-window-A + (setq wind-A ediff-window-A wind-B ediff-window-B wind-C ediff-window-C) (if arg - (progn - (ediff-with-current-buffer buffer - (setq shift (- beg pt))) - (select-window wind-A) - (if past-last-diff (goto-char (point-max))) - (condition-case nil - (backward-char shift) ; noerror, if beginning of buffer - (error)) - (recenter) - (select-window wind-B) - (if past-last-diff (goto-char (point-max))) - (condition-case nil - (backward-char shift) ; noerror, if beginning of buffer - (error)) - (recenter) - (if (window-live-p wind-C) - (progn - (select-window wind-C) - (if past-last-diff (goto-char (point-max))) - (condition-case nil - (backward-char shift) ; noerror, if beginning of buffer - (error)) - (recenter) - )) - (select-window ctl-wind) - )) - )) - + (save-selected-window + (setq shift (- beg pt)) + (ediff-with-live-window wind-A + (when past-last-diff (goto-char (point-max))) + (ignore-errors (backward-char shift)) + (recenter)) + (ediff-with-live-window wind-B + (when past-last-diff (goto-char (point-max))) + (ignore-errors (backward-char shift)) + (recenter)) + (ediff-with-live-window wind-C + (when past-last-diff (goto-char (point-max))) + (ignore-errors (backward-char shift)) + (recenter)))))) ;; find region most related to the current point position (or POS, if given) ;; returns diff number as seen by the user (i.e., 1+ the internal @@ -2725,10 +2700,7 @@ only if this merge job is part of a group, i.e., was invoked from within (let* ((buf-A ediff-buffer-A) (buf-B ediff-buffer-B) (buf-C ediff-buffer-C) - (buf-A-wind (ediff-get-visible-buffer-window buf-A)) - (buf-B-wind (ediff-get-visible-buffer-window buf-B)) - (buf-C-wind (ediff-get-visible-buffer-window buf-C)) - (buf-patch (if (boundp 'ediff-patchbufer) ediff-patchbufer nil)) + (buf-patch (if (boundp 'ediff-patchbufer) ediff-patchbufer nil)) (buf-patch-diag (if (boundp 'ediff-patch-diagnostics) ediff-patch-diagnostics nil)) (buf-err ediff-error-buffer) @@ -2746,35 +2718,18 @@ only if this merge job is part of a group, i.e., was invoked from within (if buf-fine-diff (bury-buffer buf-fine-diff)) (if buf-patch (bury-buffer buf-patch)) (if buf-patch-diag (bury-buffer buf-patch-diag)) - (if (window-live-p buf-A-wind) - (progn - (select-window buf-A-wind) - (delete-other-windows) - (bury-buffer)) - (if (ediff-buffer-live-p buf-A) - (progn - (set-buffer buf-A) - (bury-buffer)))) - (if (window-live-p buf-B-wind) - (progn - (select-window buf-B-wind) - (delete-other-windows) - (bury-buffer)) - (if (ediff-buffer-live-p buf-B) - (progn - (set-buffer buf-B) - (bury-buffer)))) - (if (window-live-p buf-C-wind) - (progn - (select-window buf-C-wind) - (delete-other-windows) - (bury-buffer)) - (if (ediff-buffer-live-p buf-C) - (progn - (set-buffer buf-C) - (bury-buffer)))) - )) - + (cl-loop + with buffers = (list buf-A buf-B buf-C) + with windows = (mapcar #'ediff-get-visible-buffer-window buffers) + for buffer in buffers + for window in windows + do (cond ((window-live-p window) + (select-window window) + (delete-other-windows) + (bury-buffer)) + (buffer + (set-buffer buffer) + (bury-buffer)))))) (defun ediff-suspend () "Suspend Ediff. @@ -3274,8 +3229,9 @@ Without an argument, it saves customized diff argument, if available (setq ediff-temp-indirect-buffer t)) (pop-to-buffer cloned-buff) (setq wind (ediff-get-visible-buffer-window cloned-buff)) - (select-window wind) - (delete-other-windows) + (when (window-live-p wind) + (select-window wind) + (delete-other-windows)) (or (mark) (push-mark)) (setq mark-active 'ediff-util) (setq-local transient-mark-mode t) @@ -3310,7 +3266,8 @@ Without an argument, it saves customized diff argument, if available (let ((cloned-buff (ediff-make-cloned-buffer buff region-name))) (ediff-with-current-buffer cloned-buff (setq ediff-temp-indirect-buffer t)) - (set-window-buffer wind cloned-buff) + (when (window-live-p wind) + (set-window-buffer wind cloned-buff)) cloned-buff)) (defun ediff-buffer-type (buffer) diff --git a/lisp/vc/ediff-wind.el b/lisp/vc/ediff-wind.el index 4ac21cb4136..aefa17de2c6 100644 --- a/lisp/vc/ediff-wind.el +++ b/lisp/vc/ediff-wind.el @@ -53,16 +53,21 @@ frame. If you don't like any of the two provided functions, write your own one. The basic guidelines: - 1. It should leave the control buffer current and the control window - selected. + 1. It should leave the control buffer current and, if showing, + the control window selected if showing these windows. 2. It should set `ediff-window-A', `ediff-window-B', `ediff-window-C', and `ediff-control-window' to contain window objects that display - the corresponding buffers. + the corresponding buffers or `nil' if the corresponding window + is not shown. 3. It should accept the following arguments: buffer-A, buffer-B, buffer-C, control-buffer Buffer C may not be used in jobs that compare only two buffers. If you plan to do something fancy, take a close look at how the two -provided functions are written." +provided functions are written. + +Set `ediff-select-control-window-on-setup' to nil to prevent the window +`ediff-control-window' being selected by ediff after this +function returns. " :type '(choice (const :tag "Choose Automatically" ediff-setup-windows-default) (const :tag "Multi Frame" ediff-setup-windows-multiframe) (const :tag "Single Frame" ediff-setup-windows-plain) @@ -247,6 +252,17 @@ keyboard input to go into icons." ;;; Functions +(defmacro ediff-with-live-window (window &rest body) + "Like `with-selected-window' but only if WINDOW is live. +If WINDOW is not live (or not a window) do nothing and don't evaluate +BODY, instead returning nil." + (declare (indent 1) (debug (form body))) + (let ((w (gensym "window"))) + `(let ((,w ,window)) + (when (window-live-p ,w) + (with-selected-window ,w + ,@body))))) + (defun ediff-get-window-by-clicking (_wind _prev-wind wind-number) (let (event) (message -- 2.39.5