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

[Spencer Baugh <sbaugh@janestreet.com>]

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

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: luangruo@yahoo.com,  sbaugh@catern.com,  62164@debbugs.gnu.org
>> Date: Mon, 05 Jun 2023 17:56:38 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> >> Cc: Po Lu <luangruo@yahoo.com>, 62164@debbugs.gnu.org
>> >> From: Spencer Baugh <sbaugh@janestreet.com>
>> >> Date: Tue, 09 May 2023 14:27:11 -0400
>> >> 
>> >> +(defcustom ediff-x-utility-control-frame nil
>> >> +  "If non-nil, the control frame is a utility window under X.
>> >> +
>> >> +This is useful in tiling window managers, where this will cause
>> >> +the control frame to be floating rather than tiled.  It should be
>> >> +harmless on other well-behaved window managers."
>> >
>> > If this option causes the control frame to be floating rather than
>> > tiled, the name of the option should reflect that.  Using "utility" in
>> > the name of the variable makes the option less self-explanatory,
>> > because that technical terms is not relevant on the user level.
>> >
>> > Thanks.
>> 
>> Agreed. Revised patch attached:
>
> Thanks, but I think the first line of the doc string should also be
> modified to explain better what the option does.  The rest of the doc
> string could then mention "utility window", if doing so will help
> someone to understand what happens and why.  But the first line should
> be a concise summary, because the various apropos commands show only
> that one line.
>
>>      (modify-frame-parameters ctl-frame adjusted-parameters)
>> +    (when (and ediff-x-floating-control-frame (eq window-system 'x))
>> +      (ediff-frame-make-utility ctl-frame))       ^^^^^^^^^^^^^
>
> I think it is better to use window-system the function here, since
> you are talking about a specific frame.

OK, revised patch attached.

I don't know how to name the variable to incorporate both you and Po's
feedback.  The name of the variable can either include "utility" to
describe its implementation, or include "floating" to describe its
effect on most window managers.  (Or include neither, and describe
nothing, which seems worse.)

I personally prefer your variable name, since I still don't know of any
window managers that don't float utility windows, but I don't care much.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-setting-the-ediff-control-frame-as-a-utility-w.patch --]
[-- Type: text/x-patch, Size: 2513 bytes --]

From b8f893709f914b483997791507ee2db888bb9279 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Tue, 9 May 2023 14:22:29 -0400
Subject: [PATCH] Allow setting the ediff control frame as a utility window
 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-x-utility-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)
---
 lisp/vc/ediff-wind.el | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/lisp/vc/ediff-wind.el b/lisp/vc/ediff-wind.el
index eb903f093f9..7471c5f0fd3 100644
--- a/lisp/vc/ediff-wind.el
+++ b/lisp/vc/ediff-wind.el
@@ -69,6 +69,17 @@ ediff-window-setup-function
 		 (function :tag "Other function"))
   :version "24.3")
 
+(defcustom ediff-x-floating-control-frame nil
+  "If non-nil, the control frame will float under most X WMs.
+
+The control frame will be give the window type \"utility\".  On
+most tiling window managers, this will cause the control frame to
+be floating rather than tiled.  It should be harmless on other
+well-behaved window managers."
+  :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 +884,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 +969,8 @@ ediff-setup-control-frame
     (goto-char (point-min))
 
     (modify-frame-parameters ctl-frame adjusted-parameters)
+    (when (and ediff-x-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.39.3