bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Drew Adams]

Possible enhancement: Change default binding of `C-x 5 2' to
`clone-frame'.  Unlike `make-frame-command', `clone-frame' would use the
same frame-parameter values as the selected frame.  Here's a possible
definition of `clone-frame':

(defun clone-frame (&optional frame)
  "Make a new frame with the same parameters as FRAME.
FRAME defaults to the selected frame.  The frame is created on the
same terminal as FRAME.  If the terminal is a text-only terminal then
also select the new frame."
  (interactive)
  (let* ((default-frame-alist  (frame-parameters frame))
         (new-fr  (make-frame)))
    (unless (display-graphic-p) (select-frame new-fr))))

In GNU Emacs 26.1 (build 1, x86_64-w64-mingw32)
 of 2018-05-30
Repository revision: 07f8f9bc5a51f5aa94eb099f3e15fbe0c20ea1ea
Windowing system distributor `Microsoft Corp.', version 10.0.16299
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Robert Pluim]

Drew Adams <drew.adams@oracle.com> writes:

> Possible enhancement: Change default binding of `C-x 5 2' to
> `clone-frame'.  Unlike `make-frame-command', `clone-frame' would use the
> same frame-parameter values as the selected frame.  Here's a possible
> definition of `clone-frame':

Hmm, how common is it to have a frame that doesnʼt have the same frame
parameters as 'default-frame-alist', and you want to replicate them?
Personally I quite like the current behaviour.

> (defun clone-frame (&optional frame)
>   "Make a new frame with the same parameters as FRAME.
> FRAME defaults to the selected frame.  The frame is created on the
> same terminal as FRAME.  If the terminal is a text-only terminal then
> also select the new frame."
>   (interactive)
>   (let* ((default-frame-alist  (frame-parameters frame))
>          (new-fr  (make-frame)))
>     (unless (display-graphic-p) (select-frame new-fr))))

How about you make it take a prefix argument to get back
'make-frame-command' semantics (or keep the current ones, and add
'clone-frame' semantics to the prefix arg)?

Robert

bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Drew Adams]

> > Possible enhancement: Change default binding of `C-x 5 2' to
> > `clone-frame'.  Unlike `make-frame-command', `clone-frame' would use the
> > same frame-parameter values as the selected frame.  Here's a possible
> > definition of `clone-frame':
> 
> Hmm, how common is it to have a frame that doesnʼt have the same frame
> parameters as 'default-frame-alist', and you want to replicate them?
> Personally I quite like the current behaviour.

It's definitely the case for me. I use several kinds of special-display frames. If I use it on a *Help* frame get a *Help* frame (same background etc.). If I use it on an *info* frame I get an *info* frame, and so on.

If it's not the case for you then you should get pretty much the same behavior as now. I don't think you'll notice a difference.

> How about you make it take a prefix argument to get back
> 'make-frame-command' semantics (or keep the current ones, and add
> 'clone-frame' semantics to the prefix arg)?

Fair enough.

(defun clone-frame (&optional frame no-clone)
  "Make a new frame with the same parameters as FRAME.
With a prefix arg, don't clone - just call `make-frame-command'.

FRAME defaults to the selected frame.  The frame is created on the
same terminal as FRAME.  If the terminal is a text-only terminal then
also select the new frame."
  (interactive "i\nP")
  (if no-clone
      (make-frame-command)
    (let* ((default-frame-alist  (frame-parameters frame))
           (new-fr  (make-frame)))
      (unless (if (fboundp 'display-graphic-p) (display-graphic-p) window-system)
        (select-frame new-fr)))))

bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Phil Sainty]

On 15/09/18 06:57, Robert Pluim wrote:
> How about you make it take a prefix argument to get back
> 'make-frame-command' semantics (or keep the current ones, and add
> 'clone-frame' semantics to the prefix arg)?

I don't have an opinion on which behaviour should be the default,
but I like the idea of adding such prefix argument behaviour to the
C-x 5 2 binding so that users can choose between the two, so +1 to
that idea in general.

-Phil

bug#34715: 26.1; (1) Add `clone-frame', (2) bind it to `C-x 5 2'

[Drew Adams]

Enhancement request:

1. Add a `clone-frame' command such as this one, which is similar to
   what I've been using:

(defun clone-frame (&optional frame no-clone)
  "Make a new frame with the same parameters as FRAME.
With a prefix arg, don't clone - just call `make-frame-command'.

FRAME defaults to the selected frame.  The frame is created on the
same terminal as FRAME.  If the terminal is a text-only terminal then
also select the new frame."
  (interactive "i\nP")
  (if no-clone
      (make-frame-command)
    (let* ((default-frame-alist  (frame-parameters frame))
           (new-fr               (make-frame)))
      (unless (display-graphic-p) (select-frame new-fr))
      new-fr)))

2. Use it, not `make-frame-command', as the binding of `C-x 5 2'.

   Why change the default behavior of `C-x 5 2'?  If I want the buffer
   of the selected window shown in another frame then I typically want
   that frame to have the same parameters.

   It's true that I often have only one window in the selected frame,
   which is not typical of other users, and in that case it perhaps
   makes even more sense that I want `C-x 5 2' to duplicate the frame's
   parameters.  But I think that most users are likely to prefer, as the
   default behavior, keeping the same parameters.  (`C-u' gives
   `make-frame-command', i.e., the frame parameters are not cloned.)  An
   open question, I guess.

3. BTW, I think it would be good to add this to the doc string of
   `make-frame-command':

   Return the new frame.

In GNU Emacs 26.1 (build 1, x86_64-w64-mingw32)
 of 2018-05-30
Repository revision: 07f8f9bc5a51f5aa94eb099f3e15fbe0c20ea1ea
Windowing system distributor `Microsoft Corp.', version 10.0.17134
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

bug#34715: 26.1; (1) Add `clone-frame', (2) bind it to `C-x 5 2'

[Eli Zaretskii]

> Date: Sat, 2 Mar 2019 16:47:47 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> 
> 1. Add a `clone-frame' command such as this one, which is similar to
>    what I've been using:
> 
> (defun clone-frame (&optional frame no-clone)
>   "Make a new frame with the same parameters as FRAME.
> With a prefix arg, don't clone - just call `make-frame-command'.
> 
> FRAME defaults to the selected frame.  The frame is created on the
> same terminal as FRAME.  If the terminal is a text-only terminal then
> also select the new frame."
>   (interactive "i\nP")
>   (if no-clone
>       (make-frame-command)
>     (let* ((default-frame-alist  (frame-parameters frame))
>            (new-fr               (make-frame)))
>       (unless (display-graphic-p) (select-frame new-fr))
>       new-fr)))
> 
> 2. Use it, not `make-frame-command', as the binding of `C-x 5 2'.

I'm okay with adding a new command, but rebinding "C-x 5 2" by default
at the same time is a non-starter.  We should first let people use the
new command, and should see how many of them ask to change the default
binding.

>    Why change the default behavior of `C-x 5 2'?  If I want the buffer
>    of the selected window shown in another frame then I typically want
>    that frame to have the same parameters.

That's what default-frame-alist is for.  If you are used to change the
parameters of your frames a lot during their lifetime, which
presumably means each of your frames might look and work differently,
it is not entirely clear to me that "C-x 5 2" should produce a clone
of the random frame where you just happened to type the command.  It
could even cause trouble/unexpected behavior, with some exotic
parameters, at least in principle.

So I think we definitely should collect more experience before
changing this veteran binding.

> 3. BTW, I think it would be good to add this to the doc string of
>    `make-frame-command':
> 
>    Return the new frame.

"When called from Lisp, return the new frame."

bug#34715: 26.1; (1) Add `clone-frame', (2) bind it to `C-x 5 2'

[Drew Adams]

> > 2. Use it, not `make-frame-command', as the binding of `C-x 5 2'.
> 
> I'm okay with adding a new command, but rebinding "C-x 5 2" by default
> at the same time is a non-starter.  We should first let people use the
> new command, and should see how many of them ask to change the default
> binding.

Juri suggested binding `clone-frame' to `C-x 5 c'.
That's OK too.

I suggested in my second mail that the use of a
prefix arg could be flipped, so this would then
become a redefinition of `make-frame-command',
where a prefix arg causes cloning instead of
being a no-op (just creating a frame using
`default-frame-alist').

So that's another possibility, which is
backward-compatible (except that a prefix arg
would no longer be a no-op, as it is now).

I personally think the cloning case is more useful
than the make-a-default-frame case.

If `make-frame-command' were updated to just let a
prefix arg clone the selected frame then cloning
would be less discoverable than if the name were
`clone-frame'.

> >    Why change the default behavior of `C-x 5 2'?  If I want the
> >    buffer of the selected window shown in another frame then I
> >    typically want that frame to have the same parameters.
> 
> That's what default-frame-alist is for.

I already have what I need for my own use.  Here
I'm proposing something for Emacs - that's the
point of this enhancement.

> If you are used to change the
> parameters of your frames a lot during their lifetime, which
> presumably means each of your frames might look and work differently,
> it is not entirely clear to me that "C-x 5 2" should produce a clone
> of the random frame where you just happened to type the command.

Sorry, I don't understand your point there.

I don't just "happen to type the command" in "random
frames".  I hit its key (`C-x 5 2', for me) with a
frame selected that I want to clone.

If a frame for some reason has particular, unusual
parameters (e.g., user-defined or from some library)
then presumably a user of such a frame would be
used to its special behavior (even if s?he were
unaware of the particular frame parameters) and
would be able to decide whether cloning it is useful.

> It could even cause trouble/unexpected behavior,
> with some exotic parameters, at least in principle.

I don't see that either.  Could you give an example?
If I want to clone a frame then I want to clone that
frame.  If I don't then I don't.  Cloning includes
copying the frame parameters.

If there were a situation where it would be
problematic to copy some frame parameter then
presumably a user wouldn't ask to clone that frame.
Sure, someone could accidentally invoke the command.
But I don't see the expected unexpected harm.

And if there really were a problem then we could
add a blacklist variable or a predicate that one
could use to inhibit cloning in such a situation.

In sum, unless there is some good reason here to
fear trouble/unexpected behavior I don't see why
this enhancement is different from others.

Sure, something unexpected is always possible.
But without some real expectation of a particular
problem we should just discover it in practice
and take care of it.
 
> So I think we definitely should collect more
> experience before changing this veteran binding.

In that case I'd vote for Juri's suggestion (use
`C-x 5 c', not `C-x 5 2'), as just making cloning
available via a prefix arg for `C-x 5 2' would
make it less discoverable.

It would also be possible to do both: bind
`clone-frame' to `C-x 5 c' and let a prefix arg
to `C-x 5 2' (as `make-frame-command') invoke
`clone-frame'.

> > 3. BTW, I think it would be good to add this to the doc string of
> >    `make-frame-command':
> >
> >    Return the new frame.
> 
> "When called from Lisp, return the new frame."

It returns the frame no matter how it's called.
And only someone interested in calling it from
Lisp is interested in the return value.  But
sure, go for it, if you think it adds something.

bug#34715: 26.1; (1) Add `clone-frame', (2) bind it to `C-x 5 2'

[Eli Zaretskii]

> Date: Mon, 4 Mar 2019 09:25:20 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 34715@debbugs.gnu.org
> 
> > >    Why change the default behavior of `C-x 5 2'?  If I want the
> > >    buffer of the selected window shown in another frame then I
> > >    typically want that frame to have the same parameters.
> > 
> > That's what default-frame-alist is for.
> 
> I already have what I need for my own use.  Here
> I'm proposing something for Emacs - that's the
> point of this enhancement.

default-frame-alist is for everyone, i.e. "for Emacs".  Not just for
you and me.

> > If you are used to change the
> > parameters of your frames a lot during their lifetime, which
> > presumably means each of your frames might look and work differently,
> > it is not entirely clear to me that "C-x 5 2" should produce a clone
> > of the random frame where you just happened to type the command.
> 
> Sorry, I don't understand your point there.
> 
> I don't just "happen to type the command" in "random
> frames".  I hit its key (`C-x 5 2', for me) with a
> frame selected that I want to clone.

That's you.  Me, I type "C-x 5 2" whenever I need another frame,
regardless of the frame that happens to be selected at that time.
With your suggestion, I'll need to think which frame I want to select
before making a new one.  That's a disadvantage for me.

> > It could even cause trouble/unexpected behavior,
> > with some exotic parameters, at least in principle.
> 
> I don't see that either.  Could you give an example?

A frame parameter can be anything at all.  Cloning all of them
sometimes makes little sense.  You yourself gave an example: the
position of the frame.

> > > 3. BTW, I think it would be good to add this to the doc string of
> > >    `make-frame-command':
> > >
> > >    Return the new frame.
> > 
> > "When called from Lisp, return the new frame."
> 
> It returns the frame no matter how it's called.

But it makes no sense to talk about the return value in interactive
use, does it?

bug#34715: 26.1; (1) Add `clone-frame', (2) bind it to `C-x 5 2'

[Eli Zaretskii]

> Date: Sat, 2 Mar 2019 16:47:47 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> 
> 3. BTW, I think it would be good to add this to the doc string of
>    `make-frame-command':
> 
>    Return the new frame.

Done.

bug#34715: 26.1; (1) Add `clone-frame', (2) bind it to `C-x 5 2'

[Juri Linkov]

>> > 2. Use it, not `make-frame-command', as the binding of `C-x 5 2'.
>>
>> I'm okay with adding a new command, but rebinding "C-x 5 2" by default
>> at the same time is a non-starter.  We should first let people use the
>> new command, and should see how many of them ask to change the default
>> binding.
>
> Juri suggested binding `clone-frame' to `C-x 5 c'.
> That's OK too.

As a matter of fact, Chromium clones the current frame,
instead of using default sizes.

bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> 1. Add a `clone-frame' command such as this one, which is similar to
>    what I've been using:
>
> (defun clone-frame (&optional frame no-clone)

I've now added this to Emacs 28 (with some trivial changes) -- bound to
`C-x 5 c'.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Wed, 01 Sep 2021 11:43:34 +0200
> Cc: 34715@debbugs.gnu.org, 32736@debbugs.gnu.org
> 
> Drew Adams <drew.adams@oracle.com> writes:
> 
> > 1. Add a `clone-frame' command such as this one, which is similar to
> >    what I've been using:
> >
> > (defun clone-frame (&optional frame no-clone)
> 
> I've now added this to Emacs 28 (with some trivial changes) -- bound to
> `C-x 5 c'.

Did you try that command in a -nw session?  It has some bug with
faces.

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Did you try that command in a -nw session?  It has some bug with
> faces.

Nope; didn't try it with -nw, but I see the problem.  Any idea what's
causing that?

Here's frame-parameters in the initial frame:

((buried-buffer-list)
 (buffer-list #<buffer *scratch*> #<buffer  *Minibuf-1*>)
 (name . "F1")
 (tab-bar-lines . 0)
 (menu-bar-lines . 1)
 (unsplittable)
 (modeline . t)
 (width . 80)
 (height . 23)
 (font . "tty")
 (background-color . "unspecified-bg")
 (foreground-color . "unspecified-fg")
 (scroll-bar-foreground . "white")
 (cursor-color . "white")
 (background-mode . dark)
 (display-type . color)
 (tty . "/dev/tty")
 (tty-type . "xterm-256color")
 (minibuffer . t))

And here's the one after C-x 5 c:

((tab-bar-lines . 0)
 (menu-bar-lines . 1)
 (buried-buffer-list #<buffer *Completions*>)
 (buffer-list #<buffer *scratch*> #<buffer  *Minibuf-1*>)
 (unsplittable)
 (modeline . t)
 (width . 80)
 (height . 23)
 (font . "tty")
 (background-color . "unspecified-bg")
 (foreground-color . "unspecified-fg")
 (name . "F2")
 (minibuffer . t))



-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: drew.adams@oracle.com,  34715@debbugs.gnu.org,  32736@debbugs.gnu.org
> Date: Wed, 01 Sep 2021 14:53:05 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Did you try that command in a -nw session?  It has some bug with
> > faces.
> 
> Nope; didn't try it with -nw, but I see the problem.  Any idea what's
> causing that?

There are error messages in *Messages* that could give a clue; did you
see them?  (No, I didn't yet think about that more than 3 sec, so have
no ideas yet.)

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> There are error messages in *Messages* that could give a clue; did you
> see them?  (No, I didn't yet think about that more than 3 sec, so have
> no ideas yet.)

Ah, didn't notice the messages:

Invalid face reference: font-lock-comment-delimiter-face
Invalid face reference: font-lock-comment-face
Invalid face reference: font-lock-comment-delimiter-face
Invalid face reference: font-lock-comment-face
Invalid face reference: font-lock-comment-delimiter-face
Invalid face reference: font-lock-comment-face
Invalid face reference: font-lock-comment-delimiter-face
Invalid face reference: font-lock-comment-face
Invalid face reference: mode-line-buffer-id
Invalid face reference: minibuffer-prompt [7 times]
Invalid face reference: mode-line-buffer-id [2 times]

So basically all the fonts are gone?  Well, that's a clue; I'll try to
debug...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> So basically all the fonts are gone?  Well, that's a clue; I'll try to
> debug...

(I mean faces.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

If I filter out the `name' and `background-color' elements, then it
works.  I can kinda see the first one, but...

Hm...  no, it's `background-color' in combination with something else;
just background-color works fine.

I wonder whether this is points to a bug somewhere in the tty frame
mechanism...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 34715@debbugs.gnu.org,  32736@debbugs.gnu.org
> Date: Wed, 01 Sep 2021 15:55:29 +0200
> 
> If I filter out the `name' and `background-color' elements, then it
> works.  I can kinda see the first one, but...
> 
> Hm...  no, it's `background-color' in combination with something else;
> just background-color works fine.
> 
> I wonder whether this is points to a bug somewhere in the tty frame
> mechanism...

If the problem is background-color, why does redisplay complain about
faces saying they are *invalid*?  I could understand if it said it
cannot load some color, but why "invalid face"?

The only funky thing about background-color on TTY I can think of is
the unspecified-bg thingy.

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> If the problem is background-color, why does redisplay complain about
> faces saying they are *invalid*?  I could understand if it said it
> cannot load some color, but why "invalid face"?
>
> The only funky thing about background-color on TTY I can think of is
> the unspecified-bg thingy.

The following is the minimal case for reproduction here:

(let ((default-frame-alist
        '((background-color . "red"))))
  (make-frame))

This reliably makes all the faces go AWOL on the new frame.  (It doesn't
matter what the colour is.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#34715: bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> (let ((default-frame-alist
>         '((background-color . "red"))))
>   (make-frame))
>
> This reliably makes all the faces go AWOL on the new frame.  (It doesn't
> matter what the colour is.)

And it is indeed the problem.  If I remove this:

	  if (EQ (prop, Qforeground_color)
	      || EQ (prop, Qbackground_color))
	    update_face_from_frame_parameter (f, prop, val);

Then the faces aren't destroyed...  So the bug is in
update_face_from_frame_parameter somewhere, I think.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#34715: bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 34715@debbugs.gnu.org,  32736@debbugs.gnu.org
> Date: Wed, 01 Sep 2021 16:18:13 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > If the problem is background-color, why does redisplay complain about
> > faces saying they are *invalid*?  I could understand if it said it
> > cannot load some color, but why "invalid face"?
> >
> > The only funky thing about background-color on TTY I can think of is
> > the unspecified-bg thingy.
> 
> The following is the minimal case for reproduction here:
> 
> (let ((default-frame-alist
>         '((background-color . "red"))))
>   (make-frame))
> 
> This reliably makes all the faces go AWOL on the new frame.  (It doesn't
> matter what the colour is.)

Shouldn't we copy the parameters alist?  Maybe even deep-copy?  And
faces are supposed to be frame-local, so maybe faces also need to be
copied?  Otherwise, you are basically manipulating faces that don't
"belong" to the frame, no?  And likewise with frame's parameters
alist, I think.

bug#34715: bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> The following is the minimal case for reproduction here:
>> 
>> (let ((default-frame-alist
>>         '((background-color . "red"))))
>>   (make-frame))
>> 
>> This reliably makes all the faces go AWOL on the new frame.  (It doesn't
>> matter what the colour is.)
>
> Shouldn't we copy the parameters alist?  Maybe even deep-copy?

Do you mean in this particular test case or in `clone-frame'?  If it's
the latter, the list we get is fresh (but not very deep).

If it's this particular test case, then I don't understand what you mean.

> And faces are supposed to be frame-local, so maybe faces also need to
> be copied?  Otherwise, you are basically manipulating faces that don't
> "belong" to the frame, no?  And likewise with frame's parameters
> alist, I think.

Anyway, debugging further shows that the problem seems to be in
`frame-set-background-mode', but I've yet to isolate what exactly is
triggering the bug.

	    (dolist (face (face-list))
	      (and (not (get face 'face-override-spec))
[...]

                         (face-spec-match-p face
                                            (face-user-default-spec face)
                                            frame)))
		   (push face locally-modified-faces)))
	    ;; Now change to the new frame parameters
	    (modify-frame-parameters frame params)
	    ;; For all unmodified named faces, choose face specs
	    ;; matching the new frame parameters.
	    (dolist (face (face-list))
	      (unless (memq face locally-modified-faces)
		(face-spec-recalc face frame)))))))))

The face-spec-recalc messes up the face...  but so does the call to
face-spec-match-p, apparently?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 34715@debbugs.gnu.org,  32736@debbugs.gnu.org
> Date: Thu, 02 Sep 2021 09:44:24 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Shouldn't we copy the parameters alist?  Maybe even deep-copy?
> 
> Do you mean in this particular test case or in `clone-frame'?  If it's
> the latter, the list we get is fresh (but not very deep).

You mean, we get a fresh list because of the call to seq-filter?
(Which btw means we now need to preload seq.el?)  Is that guaranteed
to return a copy of the original alist?

And the faces aren't copied, I think, they reference the same faces
the original frame had.

bug#34715: bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> You mean, we get a fresh list because of the call to seq-filter?

No, because of this:

DEFUN ("frame-parameters", Fframe_parameters, Sframe_parameters, 0, 1, 0,
       doc: /* Return the parameters-alist of frame FRAME.

[...]
  alist = Fcopy_alist (f->param_alist);

> (Which btw means we now need to preload seq.el?)

No, it's autoloaded.

> And the faces aren't copied, I think, they reference the same faces
> the original frame had.

Right -- I don't really know how the face/frame stuff actually works.
If it was a very general problem, then surely we'd stumble across this
whenever we make a new frame.  But the bug seems to manifest only on
terminal frames -- when we change the background mode.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#34715: bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 34715@debbugs.gnu.org,  32736@debbugs.gnu.org
> Date: Thu, 02 Sep 2021 10:01:27 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > You mean, we get a fresh list because of the call to seq-filter?
> 
> No, because of this:
> 
> DEFUN ("frame-parameters", Fframe_parameters, Sframe_parameters, 0, 1, 0,
>        doc: /* Return the parameters-alist of frame FRAME.
> 
> [...]
>   alist = Fcopy_alist (f->param_alist);

Ah, right.

> > (Which btw means we now need to preload seq.el?)
> 
> No, it's autoloaded.

But frame.el is preloaded, so won't that get in the way during
bootstrap?

> > And the faces aren't copied, I think, they reference the same faces
> > the original frame had.
> 
> Right -- I don't really know how the face/frame stuff actually works.
> If it was a very general problem, then surely we'd stumble across this
> whenever we make a new frame.  But the bug seems to manifest only on
> terminal frames -- when we change the background mode.

That's true, but it doesn't mean we don't have a deeper problem.

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> > (Which btw means we now need to preload seq.el?)
>> 
>> No, it's autoloaded.
>
> But frame.el is preloaded, so won't that get in the way during
> bootstrap?

It won't get loaded until the user uses the `C-x 5 c' command.

>> > And the faces aren't copied, I think, they reference the same faces
>> > the original frame had.
>> 
>> Right -- I don't really know how the face/frame stuff actually works.
>> If it was a very general problem, then surely we'd stumble across this
>> whenever we make a new frame.  But the bug seems to manifest only on
>> terminal frames -- when we change the background mode.
>
> That's true, but it doesn't mean we don't have a deeper problem.

True.  I'll continue to debug this, but won't have time today, I
think...  If you want to have a look meanwhile, please go ahead.  :-)
It reproduces easily, and the bug seems to be caused by something called
by `frame-set-background-mode', but that may or may not be correct
(perhaps it's just doing something that triggers the real problem caused
somewhere else).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 34715@debbugs.gnu.org,  32736@debbugs.gnu.org
> Date: Thu, 02 Sep 2021 10:57:24 +0200
> 
> >> > And the faces aren't copied, I think, they reference the same faces
> >> > the original frame had.
> >> 
> >> Right -- I don't really know how the face/frame stuff actually works.
> >> If it was a very general problem, then surely we'd stumble across this
> >> whenever we make a new frame.  But the bug seems to manifest only on
> >> terminal frames -- when we change the background mode.
> >
> > That's true, but it doesn't mean we don't have a deeper problem.
> 
> True.  I'll continue to debug this, but won't have time today, I
> think...  If you want to have a look meanwhile, please go ahead.  :-)
> It reproduces easily, and the bug seems to be caused by something called
> by `frame-set-background-mode', but that may or may not be correct
> (perhaps it's just doing something that triggers the real problem caused
> somewhere else).

I think I fixed this.  The problem was indeed with letting the new
frame manipulate a separate set of faces: we arrange for that in
make-terminal-frame, but we were doing that a tad too late.

bug#34715: bug#32736: bug#34715: bug#32736: 26; Bind `C-x 5 2' to `clone-frame' by default

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I think I fixed this.  The problem was indeed with letting the new
> frame manipulate a separate set of faces: we arrange for that in
> make-terminal-frame, but we were doing that a tad too late.

Ah, I see!  And I can confirm that this fixes the problem here, too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no