bug#62164: 29.0.60; ediff behaves poorly by default on tiling window managers

[sbaugh@catern.com]

[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Wed, 28 Jun 2023 08:55:13 -0400, Spencer Baugh <sbaugh@janestreet.com> said:
>
>     Spencer> * lisp/vc/ediff-wind.el (ediff-floating-control-frame): Add
>     Spencer> defcustom.
>     Spencer> (ediff-frame-make-utility, ediff-setup-control-frame): Allow setting
>     Spencer> the control frame up as a utility window under X. (bug#62164)
>
> Two spaces after full stop. And I think the convention is to
> capitalize 'Bug', but we donʼt follow that very consistently.
>
> I think the new user option warrants an entry in etc/NEWS

Added, patch at the end.

>     Spencer> ---
>     Spencer>  lisp/vc/ediff-wind.el | 22 ++++++++++++++++++++++
>     Spencer>  1 file changed, 22 insertions(+)
>
>     Spencer> diff --git a/lisp/vc/ediff-wind.el b/lisp/vc/ediff-wind.el
>     Spencer> index eb903f093f9..74e0e1f9b9f 100644
>     Spencer> --- a/lisp/vc/ediff-wind.el
>     Spencer> +++ b/lisp/vc/ediff-wind.el
>     Spencer> @@ -69,6 +69,16 @@ ediff-window-setup-function
>     Spencer>  		 (function :tag "Other function"))
>     Spencer>    :version "24.3")
>  
>     Spencer> +(defcustom ediff-floating-control-frame nil
>     Spencer> +  "If not-nil, try making the control frame be floating rather than tiled.
>     Spencer> +
>
> Style: the description of the option should state the action that
> follows from a non-nil value:
>
> "Whether to request a floating control frame rather than a tiled one."

No, this is correct style, see (info "(elisp) Documentation Tips")

   • The documentation string for a variable that is a yes-or-no flag
     should start with words such as “Non-nil means”, to make it clear
     that all non-‘nil’ values are equivalent and indicate explicitly
     what ‘nil’ and non-‘nil’ mean.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-floating-the-ediff-control-frame-under-X.patch --]
[-- Type: text/x-patch, Size: 3213 bytes --]

From 6b347e419898ade96f459ddec9f1fda8d7dfef84 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
Date: Fri, 7 Apr 2023 17:54:06 -0400
Subject: [PATCH] Allow floating the ediff control frame under X

This is a step in the direction of making ediff behave better by
default under tiling window managers and fixing Bug#62164.

* lisp/vc/ediff-wind.el (ediff-floating-control-frame): Add
defcustom.
(ediff-frame-make-utility, ediff-setup-control-frame): Allow setting
the control frame up as a utility window under X.  (Bug#62164)
---
 etc/NEWS              | 11 +++++++++++
 lisp/vc/ediff-wind.el | 22 ++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 9e6f0c16bcd..9c99cf6c725 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2314,6 +2314,17 @@ your mail user agent.  The behavior of 'vc-prepare-patch' can be
 modified by the user options 'vc-prepare-patches-separately' and
 'vc-default-patch-addressee'.
 
+** Ediff
+
+---
+*** New user option 'ediff-floating-control-frame'.
+If non-nil, try making the control frame be floating rather than tiled.
+
+Many X tiling window managers make the Ediff control frame a tiled
+window equal in size to the main Emacs frame, which works poorly.
+This option is useful to set if you use such a window manager, and
+will be harmless (or helpful) on most other modern window managers.
+
 ** Message
 
 ---
diff --git a/lisp/vc/ediff-wind.el b/lisp/vc/ediff-wind.el
index eb903f093f9..3077c562d63 100644
--- a/lisp/vc/ediff-wind.el
+++ b/lisp/vc/ediff-wind.el
@@ -69,6 +69,16 @@ ediff-window-setup-function
 		 (function :tag "Other function"))
   :version "24.3")
 
+(defcustom ediff-floating-control-frame nil
+  "If non-nil, try making the control frame be floating rather than tiled.
+
+If your X window manager makes the Ediff control frame a tiled one,
+set this to a non-nil value, and Emacs will try to make it floating.
+This only has effect on X displays."
+  :type '(choice (const :tag "Control frame floats" t)
+                 (const :tag "Control frame has default WM behavior" nil))
+  :version "30.1")
+
 (ediff-defvar-local ediff-multiframe nil
   "Indicates if we are in a multiframe setup.")
 
@@ -873,6 +883,16 @@ ediff-window-ok-for-display
     (not (ediff-frame-has-dedicated-windows (window-frame wind)))
     )))
 
+(defun ediff-frame-make-utility (frame)
+  (let ((x-fast-protocol-requests t))
+    (x-change-window-property
+     "_NET_WM_WINDOW_TYPE" '("_NET_WM_WINDOW_TYPE_UTILITY")
+     frame "ATOM" 32 t)
+    (x-change-window-property
+     "WM_TRANSIENT_FOR"
+     (list (string-to-number (frame-parameter nil 'window-id)))
+     frame "WINDOW" 32 t)))
+
 ;; Prepare or refresh control frame
 (defun ediff-setup-control-frame (ctl-buffer designated-minibuffer-frame)
   (let ((window-min-height 1)
@@ -948,6 +968,8 @@ ediff-setup-control-frame
     (goto-char (point-min))
 
     (modify-frame-parameters ctl-frame adjusted-parameters)
+    (when (and ediff-floating-control-frame (eq (window-system ctl-frame) 'x))
+      (ediff-frame-make-utility ctl-frame))
     (make-frame-visible ctl-frame)
 
     ;; This works around a bug in 19.25 and earlier.  There, if frame gets
-- 
2.41.0