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

From: martin rudalics <[email-masked]>
Date: Mon, 4 Jan 2021 19:54:22 +0100

>  > 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 ...

 > In fact my first attempt seems to compile and behave as expected, so I
 > have a few questions on how to proceed:
 >
 > - I need to repeatedly use a pattern that looks like this:
 >
 > int is_child_frame = FRAME_PARENT_FRAME(f) != NULL;

In that case you should make is_child_frame a bool and not an int.  But
it's much simpler to just test for FRAME_PARENT_FRAME (f) wherever you
see a need for is_child_frame like in

 > int border_face_id = is_child_frame

   int border_face_id = FRAME_PARENT_FRAME (f)

 >     ? CHILD_FRAME_BORDER_FACE_ID
 >     : INTERNAL_BORDER_FACE_ID;
 > int face_id = !NILP (Vface_remapping_alist)
 >     ? lookup_basic_face (NULL, f, border_face_id)
 >     : border_face_id;
 >
 > 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));

and in lookup_basic_face in xfaces.c you'd then have to add

     case CHILD_FRAME_BORDER_FACE_ID:	name = Qchild_frame_border; 	break;

It's augmenting forms like that last one that get the most obscure bugs,
so don't despair.

 > - 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.

Note that people have already set up their own child frame creation
routines and we should try to not punish them too much.  And please try
to discuss this on the list you cited earlier and also with the
posframe.el designer.  Mister Feng Shu you've inevitably become a
participant of this thread now.

 > - 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.

martin