bug#45620: 28.0.50; Child frames should have their own border width and colour

[Alexander Miller <alexanderm@web.de>]

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

 >>> Can you propose a patch?
 >>
 >> I can *try*. I am absolutely not a C programmer, but as long as the
task
 >> is limited to a monkey see, monkey do situation for handling a new face
 >> I should be able to hammer something useful together.
 >
 > That's one way to become a C programmer ...

To be honest I've don't really like C as a language, and learning it
would not do anything professionally for me either. So I'm jumping into
this strictly for the sake of improving Emacs.

 >> and I would like to put it into a single function accessible from
 >> anywhere. Is that the right call, and if yes, where would be the right
 >> place to put it?
 >
 > This is the first time I see the internal border face getting remapped.
 > I wasn't aware that nsterm.c does that and I'm not sure whether we
 > should add something similar to xterm.c and w32term.c. In nsterm.c I
 > would not write an extra function but instead of what we have now use
 >
 > int face_id =
 > (FRAME_PARENT_FRAME (f)
 > ? (!NILP (Vface_remapping_alist)
 > ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID)
 > : CHILD_FRAME_BORDER_FACE_ID)
 > : (!NILP (Vface_remapping_alist)
 > ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
 > : INTERNAL_BORDER_FACE_ID));

x, ns and w32 all used the same code, so I changed them all to look like
this now.

 >> - Currently the actual width of the border is still controlled by the
 >> `internal-border-width` parameter for both frame types. Should I try to
 >> do something about that as well? If yes, what's my entry point?
 >
 > Add a 'child-frame-border-width' parameter. But in this case I would
 > propose to proceed as follows:
 >
 > - If for a frame the 'child-frame-border-width' was explicitly set, use
 > it.
 >
 > - If it was not set, use the 'internal-border-width' parameter.

That's done now too, at least for X. I'll attach patches of my first
working drafts for both changes. Let me know if I'm on the right path so
far.

 >> - I think I'll need to sign the FSF copyright assignment, unless the
 >> limit is higher than the 15 lines I am remembering.
 >
 > I think so too.
 >

Ok, what do I do?


[-- Attachment #2: 0001-WIP-child-frame-border.patch --]
[-- Type: text/x-patch, Size: 6405 bytes --]

From 50e3d31abf4bd12e24f82bb2f8e39ec1c3c073e0 Mon Sep 17 00:00:00 2001
From: Alexander Miller <alexanderm@web.de>
Date: Tue, 5 Jan 2021 11:33:31 +0100
Subject: [PATCH 1/2] WIP child-frame-border

---
 lisp/faces.el    | 11 ++++++++++-
 src/dispextern.h |  1 +
 src/nsterm.m     | 10 +++++++---
 src/w32fns.c     | 10 +++++++---
 src/xfaces.c     |  3 +++
 src/xterm.c      | 20 ++++++++++++++------
 6 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/lisp/faces.el b/lisp/faces.el
index 4e98338432..639dbeb09a 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2683,11 +2683,20 @@ window-divider-last-pixel

 (defface internal-border
     '((t nil))
-  "Basic face for the internal border."
+  "Basic face for the internal border.
+For the internal border of child frames see `child-frame-border'."
   :version "26.1"
   :group 'frames
   :group 'basic-faces)

+(defface child-frame-border
+  '((t nil))
+  "Basic face for the internal border of child frames.
+For the internal border of non-child frames see `internal-border'."
+  :version "28.1"
+  :group 'frames
+  :group 'basic-faces)
+
 (defface minibuffer-prompt
   '((((background dark)) :foreground "cyan")
     ;; Don't use blue because many users of the MS-DOS port customize
diff --git a/src/dispextern.h b/src/dispextern.h
index 3ad98b8344..f4e872644d 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1826,6 +1826,7 @@ #define FACE_UNIBYTE_P(FACE) ((FACE)->charset < 0)
   WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID,
   WINDOW_DIVIDER_LAST_PIXEL_FACE_ID,
   INTERNAL_BORDER_FACE_ID,
+  CHILD_FRAME_BORDER_FACE_ID,
   TAB_BAR_FACE_ID,
   TAB_LINE_FACE_ID,
   BASIC_FACE_ID_SENTINEL
diff --git a/src/nsterm.m b/src/nsterm.m
index 2defb9e2ee..76f67d2531 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3029,9 +3029,13 @@ so some key presses (TAB) are swallowed by the system.  */
       NSRectEdge edge[] = {NSMinXEdge, NSMinYEdge, NSMaxXEdge, NSMaxYEdge};

       int face_id =
-	!NILP (Vface_remapping_alist)
-	? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
-	: INTERNAL_BORDER_FACE_ID;
+        (FRAME_PARENT_FRAME (f)
+         ? (!NILP (Vface_remapping_alist)
+            ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID)
+            : CHILD_FRAME_BORDER_FACE_ID)
+         : (!NILP (Vface_remapping_alist)
+            ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
+            : INTERNAL_BORDER_FACE_ID));
       struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);

       if (!face)
diff --git a/src/w32fns.c b/src/w32fns.c
index c1e18ff7fa..061886f0bc 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -1519,9 +1519,13 @@ w32_clear_under_internal_border (struct frame *f)
       int width = FRAME_PIXEL_WIDTH (f);
       int height = FRAME_PIXEL_HEIGHT (f);
       int face_id =
-	!NILP (Vface_remapping_alist)
-	? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
-	: INTERNAL_BORDER_FACE_ID;
+	(FRAME_PARENT_FRAME (f)
+	 ? (!NILP (Vface_remapping_alist)
+	    ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID)
+	    : CHILD_FRAME_BORDER_FACE_ID)
+	 : (!NILP (Vface_remapping_alist)
+	    ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
+	    : INTERNAL_BORDER_FACE_ID));
       struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);

       block_input ();
diff --git a/src/xfaces.c b/src/xfaces.c
index b3b19a9cb2..5080120957 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -4913,6 +4913,7 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id)
     case WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID:	name = Qwindow_divider_first_pixel;	break;
     case WINDOW_DIVIDER_LAST_PIXEL_FACE_ID:	name = Qwindow_divider_last_pixel;	break;
     case INTERNAL_BORDER_FACE_ID:	name = Qinternal_border; 	break;
+    case CHILD_FRAME_BORDER_FACE_ID:	name = Qchild_frame_border; 	break;

     default:
       emacs_abort (); /* the caller is supposed to pass us a basic face id */
@@ -5619,6 +5620,7 @@ realize_basic_faces (struct frame *f)
       realize_named_face (f, Qwindow_divider_last_pixel,
 			  WINDOW_DIVIDER_LAST_PIXEL_FACE_ID);
       realize_named_face (f, Qinternal_border, INTERNAL_BORDER_FACE_ID);
+      realize_named_face (f, Qchild_frame_border, CHILD_FRAME_BORDER_FACE_ID);
       realize_named_face (f, Qtab_bar, TAB_BAR_FACE_ID);
       realize_named_face (f, Qtab_line, TAB_LINE_FACE_ID);

@@ -6967,6 +6969,7 @@ syms_of_xfaces (void)
   DEFSYM (Qwindow_divider_first_pixel, "window-divider-first-pixel");
   DEFSYM (Qwindow_divider_last_pixel, "window-divider-last-pixel");
   DEFSYM (Qinternal_border, "internal-border");
+  DEFSYM (Qchild_frame_border, "child-frame-border");

   /* TTY color-related functions (defined in tty-colors.el).  */
   DEFSYM (Qtty_color_desc, "tty-color-desc");
diff --git a/src/xterm.c b/src/xterm.c
index 0a86738cc2..92dcd95043 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -1293,9 +1293,13 @@ x_clear_under_internal_border (struct frame *f)
       int height = FRAME_PIXEL_HEIGHT (f);
       int margin = FRAME_TOP_MARGIN_HEIGHT (f);
       int face_id =
-	!NILP (Vface_remapping_alist)
-	? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
-	: INTERNAL_BORDER_FACE_ID;
+	(FRAME_PARENT_FRAME (f)
+	 ? (!NILP (Vface_remapping_alist)
+	    ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID)
+	    : CHILD_FRAME_BORDER_FACE_ID)
+	 : (!NILP (Vface_remapping_alist)
+	    ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
+	    : INTERNAL_BORDER_FACE_ID));
       struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);

       block_input ();
@@ -1360,9 +1364,13 @@ x_after_update_window_line (struct window *w, struct glyph_row *desired_row)
       {
 	int y = WINDOW_TO_FRAME_PIXEL_Y (w, max (0, desired_row->y));
 	int face_id =
-	  !NILP (Vface_remapping_alist)
-	  ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
-	  : INTERNAL_BORDER_FACE_ID;
+	  (FRAME_PARENT_FRAME (f)
+	   ? (!NILP (Vface_remapping_alist)
+	      ? lookup_basic_face (NULL, f, CHILD_FRAME_BORDER_FACE_ID)
+	      : CHILD_FRAME_BORDER_FACE_ID)
+	   : (!NILP (Vface_remapping_alist)
+	      ? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
+	      : INTERNAL_BORDER_FACE_ID));
 	struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);

 	block_input ();
--
2.29.2


[-- Attachment #3: 0002-WIP-child-frame-border-width.patch --]
[-- Type: text/x-patch, Size: 5925 bytes --]

From b176909995eea600d99354be2127469f472ce4bb Mon Sep 17 00:00:00 2001
From: Alexander Miller <alexanderm@web.de>
Date: Tue, 5 Jan 2021 13:31:09 +0100
Subject: [PATCH 2/2] WIP child-frame-border-width

---
 src/frame.c |  4 ++++
 src/frame.h | 14 +++++++++++++-
 src/xfns.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/src/frame.c b/src/frame.c
index 45ee96e962..d5040dfae6 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -3744,6 +3744,7 @@ DEFUN ("set-frame-window-state-change", Fset_frame_window_state_change,
   {"foreground-color",		-1},
   {"icon-name",			SYMBOL_INDEX (Qicon_name)},
   {"icon-type",			SYMBOL_INDEX (Qicon_type)},
+  {"child-frame-border-width",	SYMBOL_INDEX (Qchild_frame_border_width)},
   {"internal-border-width",	SYMBOL_INDEX (Qinternal_border_width)},
   {"right-divider-width",	SYMBOL_INDEX (Qright_divider_width)},
   {"bottom-divider-width",	SYMBOL_INDEX (Qbottom_divider_width)},
@@ -4287,6 +4288,8 @@ gui_report_frame_params (struct frame *f, Lisp_Object *alistptr)

   store_in_alist (alistptr, Qborder_width,
 		  make_fixnum (f->border_width));
+  store_in_alist (alistptr, Qchild_frame_border_width,
+		  make_fixnum (CHILD_FRAME_BORDER_WIDTH (f)));
   store_in_alist (alistptr, Qinternal_border_width,
 		  make_fixnum (FRAME_INTERNAL_BORDER_WIDTH (f)));
   store_in_alist (alistptr, Qright_divider_width,
@@ -5984,6 +5987,7 @@ syms_of_frame (void)
   DEFSYM (Qhorizontal_scroll_bars, "horizontal-scroll-bars");
   DEFSYM (Qicon_name, "icon-name");
   DEFSYM (Qicon_type, "icon-type");
+  DEFSYM (Qchild_frame_border_width, "child-frame-border-width");
   DEFSYM (Qinternal_border_width, "internal-border-width");
   DEFSYM (Qleft_fringe, "left-fringe");
   DEFSYM (Qline_spacing, "line-spacing");
diff --git a/src/frame.h b/src/frame.h
index 8cf41dc004..7d816f2516 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -534,6 +534,10 @@ #define EMACS_FRAME_H
   /* Border width of the frame window as known by the (X) window system.  */
   int border_width;

+  /* Width of child frames' internal border.  Acts exactly like
+     internal_border_width below and will fall back on it when not set.*/
+  int child_frame_border_width;
+
   /* Width of the internal border.  This is a line of background color
      just inside the window's border.  When the frame is selected,
      a highlighting is displayed inside the internal border.  */
@@ -1432,11 +1436,19 @@ FRAME_TOTAL_FRINGE_WIDTH (struct frame *f)
   return FRAME_LEFT_FRINGE_WIDTH (f) + FRAME_RIGHT_FRINGE_WIDTH (f);
 }

+INLINE int
+CHILD_FRAME_BORDER_WIDTH (struct frame *f)
+{
+  return frame_dimension (f->child_frame_border_width);
+}
+
 /* Pixel-width of internal border lines.  */
 INLINE int
 FRAME_INTERNAL_BORDER_WIDTH (struct frame *f)
 {
-  return frame_dimension (f->internal_border_width);
+  return f->child_frame_border_width
+    ? CHILD_FRAME_BORDER_WIDTH(f)
+    : frame_dimension (f->internal_border_width);
 }

 /* Pixel-size of window divider lines.  */
diff --git a/src/xfns.c b/src/xfns.c
index 9ab537ca8d..663146d7f2 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -1800,6 +1800,28 @@ x_change_tool_bar_height (struct frame *f, int height)
 #endif /* USE_GTK */
 }

+static void
+x_set_child_frame_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
+{
+  int border = check_int_nonnegative (arg);
+
+  if (border != CHILD_FRAME_BORDER_WIDTH (f))
+    {
+      f->child_frame_border_width = border;
+
+#ifdef USE_X_TOOLKIT
+      if (FRAME_X_OUTPUT (f)->edit_widget)
+	widget_store_internal_border (FRAME_X_OUTPUT (f)->edit_widget);
+#endif
+
+      if (FRAME_X_WINDOW (f))
+	{
+	  adjust_frame_size (f, -1, -1, 3, false, Qchild_frame_border_width);
+	  x_clear_under_internal_border (f);
+	}
+    }
+
+}

 static void
 x_set_internal_border_width (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
@@ -3897,14 +3919,37 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
 	parms = Fcons (Fcons (Qinternal_border_width, value),
 		       parms);
     }
+  /* Same for child frames.  */
+  if (NILP (Fassq (Qchild_frame_border_width, parms)))
+    {
+      Lisp_Object value;
+
+      value = gui_display_get_arg (dpyinfo, parms, Qchild_frame_border_width,
+                                   "internalBorder", "internalBorder",
+                                   RES_TYPE_NUMBER);
+      if (! EQ (value, Qunbound))
+	parms = Fcons (Fcons (Qchild_frame_border_width, value),
+		       parms);
+
+    }
+
+  gui_default_parameter (f, parms, Qchild_frame_border_width,
+#ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets.  */
+			 make_fixnum (0),
+#else
+			 make_fixnum (1),
+#endif
+			 "childFrameBorderWidth", "childFrameBorderWidth",
+			 RES_TYPE_NUMBER);
   gui_default_parameter (f, parms, Qinternal_border_width,
 #ifdef USE_GTK /* We used to impose 0 in xg_create_frame_widgets.  */
-                         make_fixnum (0),
+			 make_fixnum (0),
 #else
-                         make_fixnum (1),
+			 make_fixnum (1),
 #endif
-                         "internalBorderWidth", "internalBorderWidth",
-                         RES_TYPE_NUMBER);
+			 "internalBorderWidth", "internalBorderWidth",
+			 RES_TYPE_NUMBER);
+
   gui_default_parameter (f, parms, Qright_divider_width, make_fixnum (0),
                          NULL, NULL, RES_TYPE_NUMBER);
   gui_default_parameter (f, parms, Qbottom_divider_width, make_fixnum (0),
@@ -7762,6 +7807,7 @@ DEFUN ("x-gtk-debug", Fx_gtk_debug, Sx_gtk_debug, 1, 1, 0,
   x_set_foreground_color,
   x_set_icon_name,
   x_set_icon_type,
+  x_set_child_frame_border_width,
   x_set_internal_border_width,
   gui_set_right_divider_width,
   gui_set_bottom_divider_width,
--
2.29.2