GNU bug report logs - #45620
28.0.50; Child frames should have their own border width and colour

Previous Next

Full log

View this message in rfc822 format

From: martin rudalics <rudalics <at> gmx.at>
To: Alexander Miller <alexanderm <at> web.de>
Cc: 45620 <at> debbugs.gnu.org, Feng Shu <tumashu <at> 163.com>
Subject: bug#45620: 28.0.50; Child frames should have their own border width and colour
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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.