"Final" version of tty child frames

"Final" version of tty child frames

[Gerd Möllmann]

I have (re-)created the scratch/tty-child-frames branch today, which
contains the code for child frames on ttys, based on a recent master.

I'm a happy user of this for a while now with corfu, vertico +
vertico-posframe + consult, transient + transient-posframe,
which-key + which-key-posframe. And my current todo list is now empty,
so here it is.

To use it, redefine these two functions:

#+begin_src emacs-lisp
(defun posframe-workable-p ()
  "Test posframe workable status."
  (and (>= emacs-major-version 26)
       (not (or noninteractive
                emacs-basic-display
                (or (featurep 'tty-child-frames)
                    (not (display-graphic-p)))
                (eq (frame-parameter (selected-frame) 'minibuffer) 'only)))))))

(cl-defgeneric corfu--popup-support-p ()
  "Return non-nil if child frames are supported."
  (or (display-graphic-p)
      (featurep 'tty-child-frames)))
#+end_src

To make things look nicer you might also want to

#+begin_src emacs-lisp
(push '(tty-non-selected-cursor . t) vertico-posframe-parameters)
(push '(undecorated . nil) vertico-posframe-parameters))
(push '(undecorated . nil) transient-posframe-parameters))
#+end_src

The 'undecorated' frame parameter lets Emacs draw a border around the
child frame (default is no border). The tty-non-selected-cursor
parameter makes redisplay put the terminal cursor in a non-selected
frame which I find nice for things like consult-buffer. Which is why I
added it :-).

What's there fits my personal needs entirely. I am not interested in
full support of child frames as they exist on window systems because I
don't see child frames being used except for things like posframe and
corfu and similar. And, TBH, I don't find the tty frame code fun to
work with.

Other things not contained:

- Support for anything but termcap frames. I bet I broke MSDOS.
- Mouse support except with xterm-mouse-mode (no GPM).
- Drawing borders like it is normally done (internal border and so
  on). Tried that once, git reset --hard, won't happen.
- Tooltips. I think tooltips could relatively easily be implemented
  with child frames by copying or extracting the non-native tooltip
  code from x-show-tip or some such. From my POV, that would be best
  done by refactoring the tooltip code across window systems which I
  can't do.
- Documentation. Would only make sense to write if this goes into GNU,
  which might or might not happen, depending on, from my side, how
  much work that is.

Disclaimer: As I mentioned already in other contexts, I don't want to
be the maintainer of anything, for personal reasons.

Have fun!

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Tue, 22 Oct 2024 06:46:15 +0200
> 
> I have (re-)created the scratch/tty-child-frames branch today, which
> contains the code for child frames on ttys, based on a recent master.

Thanks.

> Disclaimer: As I mentioned already in other contexts, I don't want to
> be the maintainer of anything, for personal reasons.

We have difficulty working with "fire and forget" contributions of
this size and complexity.  Maybe Jared (CC'ed) could take a look and
clean up the branch, to prepare it for landing.  Or someone else with
interest in this stuff (Martin?).

Re: "Final" version of tty child frames

[martin rudalics]

Building complains here as

In file included from ../../src/term.c:30:
../../src/lisp.h: In function ‘Ftty_display_pixel_height’:
../../src/lisp.h:406:24: warning: ‘height’ may be used uninitialized [-Wmaybe-uninitialized]
   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
       |                        ^
../../src/term.c:4907:14: note: ‘height’ was declared here
  4907 |   int width, height;
       |              ^~~~~~
../../src/lisp.h: In function ‘Ftty_display_pixel_width’:
../../src/lisp.h:406:24: warning: ‘width’ may be used uninitialized [-Wmaybe-uninitialized]
   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
       |                        ^
../../src/term.c:4896:7: note: ‘width’ was declared here
  4896 |   int width, height;
       |       ^~~~~

I cannot test much because I hardly ever use Emacs in a terminal window.
Setting size and position of the child frame work seamlessly.  I can
move the child frame completely out of the parent (but have not tried
what happens when it completely covers the parent) or make it invisible
and visible again.

One thing I noticed is that changing the background color works only
after I changed something like the position or size.  Also I would like
to get rid of those |+- borders.  What would I have to do?

martin

Re: "Final" version of tty child frames

[Eli Zaretskii]

> Date: Tue, 22 Oct 2024 11:58:59 +0200
> Cc: emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
> Building complains here as
> 
> In file included from ../../src/term.c:30:
> ../../src/lisp.h: In function ‘Ftty_display_pixel_height’:
> ../../src/lisp.h:406:24: warning: ‘height’ may be used uninitialized [-Wmaybe-uninitialized]
>    406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
>        |                        ^
> ../../src/term.c:4907:14: note: ‘height’ was declared here
>   4907 |   int width, height;
>        |              ^~~~~~
> ../../src/lisp.h: In function ‘Ftty_display_pixel_width’:
> ../../src/lisp.h:406:24: warning: ‘width’ may be used uninitialized [-Wmaybe-uninitialized]
>    406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
>        |                        ^
> ../../src/term.c:4896:7: note: ‘width’ was declared here
>   4896 |   int width, height;
>        |       ^~~~~

I attempted to fix this, please see if the warnings are gone.

Re: "Final" version of tty child frames

[martin rudalics]

 > I attempted to fix this, please see if the warnings are gone.

They are gone.

Thanks, martin

Re: "Final" version of tty child frames

[Eli Zaretskii]

> Date: Tue, 22 Oct 2024 16:01:14 +0200
> Cc: gerd.moellmann@gmail.com, jared@finder.org, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > I attempted to fix this, please see if the warnings are gone.
> 
> They are gone.

Thanks.

Re: "Final" version of tty child frames

[Gerd Möllmann]

martin rudalics <rudalics@gmx.at> writes:

> Building complains here as
>
> In file included from ../../src/term.c:30:
> ../../src/lisp.h: In function ‘Ftty_display_pixel_height’:
> ../../src/lisp.h:406:24: warning: ‘height’ may be used uninitialized [-Wmaybe-uninitialized]
>   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
>       |                        ^
> ../../src/term.c:4907:14: note: ‘height’ was declared here
>  4907 |   int width, height;
>       |              ^~~~~~
> ../../src/lisp.h: In function ‘Ftty_display_pixel_width’:
> ../../src/lisp.h:406:24: warning: ‘width’ may be used uninitialized [-Wmaybe-uninitialized]
>   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
>       |                        ^
> ../../src/term.c:4896:7: note: ‘width’ was declared here
>  4896 |   int width, height;
>       |       ^~~~~

That's code like this:

{
  int width, height;
  tty_display_dimension (display, &width, &height);
  return make_fixnum (height);
}

So width and height are returned by the call to tty_display_dimension.
TBH, I can't make sense of the warning. FWIW, clang doesn't complain.
Don't know what's the usual way is to placate GCC here. What version is
that BTW?

> I cannot test much because I hardly ever use Emacs in a terminal window.
> Setting size and position of the child frame work seamlessly.  I can
> move the child frame completely out of the parent (but have not tried
> what happens when it completely covers the parent) or make it invisible
> and visible again.

👍

> One thing I noticed is that changing the background color works only
> after I changed something like the position or size.  

Probably a SET_FRAME_GARBAGED missing somewhere, or something like that.

> Also I would like to get rid of those |+- borders. What would I have
> to do?

If you want to get rid of the border completely add a frame parameter
(undecorated . t), Default if no no undecorated is specified, it to not
draw borders.

If you want Unicode chars instead, you could call
standard-display-unicode-special-glyphs.

Re: "Final" version of tty child frames

[Po Lu]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> martin rudalics <rudalics@gmx.at> writes:
>
>> Building complains here as
>>
>> In file included from ../../src/term.c:30:
>> ../../src/lisp.h: In function ‘Ftty_display_pixel_height’:
>> ../../src/lisp.h:406:24: warning: ‘height’ may be used uninitialized [-Wmaybe-uninitialized]
>>   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
>>       |                        ^
>> ../../src/term.c:4907:14: note: ‘height’ was declared here
>>  4907 |   int width, height;
>>       |              ^~~~~~
>> ../../src/lisp.h: In function ‘Ftty_display_pixel_width’:
>> ../../src/lisp.h:406:24: warning: ‘width’ may be used uninitialized [-Wmaybe-uninitialized]
>>   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
>>       |                        ^
>> ../../src/term.c:4896:7: note: ‘width’ was declared here
>>  4896 |   int width, height;
>>       |       ^~~~~
>
> That's code like this:
>
> {
>   int width, height;
>   tty_display_dimension (display, &width, &height);
>   return make_fixnum (height);
> }
>
> So width and height are returned by the call to tty_display_dimension.
> TBH, I can't make sense of the warning. FWIW, clang doesn't complain.
> Don't know what's the usual way is to placate GCC here. What version is
> that BTW?

Insert UNINIT after the declaration of the variable affected by these
warnings.  E.g.,

  int height UNINIT ...

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Jared Finder <jared@finder.org>,
>   emacs-devel@gnu.org
> Date: Tue, 22 Oct 2024 12:40:21 +0200
> 
> martin rudalics <rudalics@gmx.at> writes:
> 
> > Building complains here as
> >
> > In file included from ../../src/term.c:30:
> > ../../src/lisp.h: In function ‘Ftty_display_pixel_height’:
> > ../../src/lisp.h:406:24: warning: ‘height’ may be used uninitialized [-Wmaybe-uninitialized]
> >   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
> >       |                        ^
> > ../../src/term.c:4907:14: note: ‘height’ was declared here
> >  4907 |   int width, height;
> >       |              ^~~~~~
> > ../../src/lisp.h: In function ‘Ftty_display_pixel_width’:
> > ../../src/lisp.h:406:24: warning: ‘width’ may be used uninitialized [-Wmaybe-uninitialized]
> >   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
> >       |                        ^
> > ../../src/term.c:4896:7: note: ‘width’ was declared here
> >  4896 |   int width, height;
> >       |       ^~~~~
> 
> That's code like this:
> 
> {
>   int width, height;
>   tty_display_dimension (display, &width, &height);
>   return make_fixnum (height);
> }
> 
> So width and height are returned by the call to tty_display_dimension.
> TBH, I can't make sense of the warning.

I could: tty_display_dimension includes a switch without 'default',
and thus might not set width and height to any value.  I installed
what should be a fix for that, and I hope Martin will confirm that the
warning is now gone.

> FWIW, clang doesn't complain.

It probably doesn't analyze the code so thoroughly.

> > One thing I noticed is that changing the background color works only
> > after I changed something like the position or size.  
> 
> Probably a SET_FRAME_GARBAGED missing somewhere, or something like that.

If someone gives the recipe for this, I could look into it.

> > Also I would like to get rid of those |+- borders. What would I have
> > to do?
> 
> If you want to get rid of the border completely add a frame parameter
> (undecorated . t), Default if no no undecorated is specified, it to not
> draw borders.

You mean, if undecorated is not specified, the default is to _draw_
the borders, yes?  Because the examples I used do not specify
undecorated, and the borders were drawn.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  Jared Finder <jared@finder.org>,
>>   emacs-devel@gnu.org
>> Date: Tue, 22 Oct 2024 12:40:21 +0200
>> 
>> martin rudalics <rudalics@gmx.at> writes:
>> 
>> > Building complains here as
>> >
>> > In file included from ../../src/term.c:30:
>> > ../../src/lisp.h: In function ‘Ftty_display_pixel_height’:
>> > ../../src/lisp.h:406:24: warning: ‘height’ may be used uninitialized [-Wmaybe-uninitialized]
>> >   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
>> >       |                        ^
>> > ../../src/term.c:4907:14: note: ‘height’ was declared here
>> >  4907 |   int width, height;
>> >       |              ^~~~~~
>> > ../../src/lisp.h: In function ‘Ftty_display_pixel_width’:
>> > ../../src/lisp.h:406:24: warning: ‘width’ may be used uninitialized [-Wmaybe-uninitialized]
>> >   406 |     XIL ((EMACS_INT) (((EMACS_UINT) (n) << INTTYPEBITS) + Lisp_Int0))
>> >       |                        ^
>> > ../../src/term.c:4896:7: note: ‘width’ was declared here
>> >  4896 |   int width, height;
>> >       |       ^~~~~
>> 
>> That's code like this:
>> 
>> {
>>   int width, height;
>>   tty_display_dimension (display, &width, &height);
>>   return make_fixnum (height);
>> }
>> 
>> So width and height are returned by the call to tty_display_dimension.
>> TBH, I can't make sense of the warning.
>
> I could: tty_display_dimension includes a switch without 'default',
> and thus might not set width and height to any value.  I installed
> what should be a fix for that, and I hope Martin will confirm that the
> warning is now gone.

Ok, thanks. I thought that was an exhaustive switch, but apparently not.

>
>> FWIW, clang doesn't complain.
>
> It probably doesn't analyze the code so thoroughly.
>
>> > One thing I noticed is that changing the background color works only
>> > after I changed something like the position or size.  
>> 
>> Probably a SET_FRAME_GARBAGED missing somewhere, or something like that.
>
> If someone gives the recipe for this, I could look into it.
>
>> > Also I would like to get rid of those |+- borders. What would I have
>> > to do?
>> 
>> If you want to get rid of the border completely add a frame parameter
>> (undecorated . t), Default if no no undecorated is specified, it to not
>> draw borders.
>
> You mean, if undecorated is not specified, the default is to _draw_
> the borders, yes?  Because the examples I used do not specify
> undecorated, and the borders were drawn.

It's this, so if it's not undecoreated.

  if (!FRAME_UNDECORATED (child))
    {
      /* Horizontal line above.  */

The double negation always gets me confused :-).

Re: "Final" version of tty child frames

[martin rudalics]

 > Don't know what's the usual way is to placate GCC here. What version is
 > that BTW?

12.2.0

 >> Also I would like to get rid of those |+- borders. What would I have
 >> to do?
 >
 > If you want to get rid of the border completely add a frame parameter
 > (undecorated . t), Default if no no undecorated is specified, it to not
 > draw borders.

Thanks.  I'm not quite sure whether these should not be called the
internal borders.  Decorations are more - on GUIs they include the title
bar.

Two further things I noticed: When point in the parent frame is
effectively hidden by the child frame, its cursor sometimes appears at
the right of the child frame and sometimes it's not shown.  I have not
understood the underlying principle for this behavior.  Also when the
selected region in the parent frame is active, its overlay covers the
child frame.  That's ugly.

martin

Re: "Final" version of tty child frames

[Jared Finder]

On 2024-10-21 22:29, Eli Zaretskii wrote:
>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Date: Tue, 22 Oct 2024 06:46:15 +0200
>> 
>> I have (re-)created the scratch/tty-child-frames branch today, which
>> contains the code for child frames on ttys, based on a recent master.
> 
> Thanks.
> 
>> Disclaimer: As I mentioned already in other contexts, I don't want to
>> be the maintainer of anything, for personal reasons.
> 
> We have difficulty working with "fire and forget" contributions of
> this size and complexity.  Maybe Jared (CC'ed) could take a look and
> clean up the branch, to prepare it for landing.  Or someone else with
> interest in this stuff (Martin?).

I'm happy to be considered here and would be interested in helping out, 
but it would have to wait. I'm extremely busy at my job for the next 
month or so and unfortunately do not have time to take a contribution 
such as this to completion.

I'll check back in when my time clears up. I would certainly be 
interested in helping here if needed. I am certainly interested in 
making TTY Emacs more functional in general.

   -- MJF

Re: "Final" version of tty child frames

[Gerd Möllmann]

Jared Finder <jared@finder.org> writes:

> On 2024-10-21 22:29, Eli Zaretskii wrote:
>>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>> Date: Tue, 22 Oct 2024 06:46:15 +0200
>>> I have (re-)created the scratch/tty-child-frames branch today,
>>> which
>>> contains the code for child frames on ttys, based on a recent master.
>> Thanks.
>> 
>>> Disclaimer: As I mentioned already in other contexts, I don't want to
>>> be the maintainer of anything, for personal reasons.
>> We have difficulty working with "fire and forget" contributions of
>> this size and complexity.  Maybe Jared (CC'ed) could take a look and
>> clean up the branch, to prepare it for landing.  Or someone else with
>> interest in this stuff (Martin?).
>
> I'm happy to be considered here and would be interested in helping
> out, but it would have to wait. I'm extremely busy at my job for the
> next month or so and unfortunately do not have time to take a
> contribution such as this to completion.
>
> I'll check back in when my time clears up. I would certainly be
> interested in helping here if needed. I am certainly interested in
> making TTY Emacs more functional in general.

👍

Re: "Final" version of tty child frames

[Eli Zaretskii]

> Date: Sun, 27 Oct 2024 21:35:29 -0700
> From: Jared Finder <jared@finder.org>
> Cc: Gerd Möllmann <gerd.moellmann@gmail.com>,
>  emacs-devel@gnu.org, martin rudalics <rudalics@gmx.at>
> 
> On 2024-10-21 22:29, Eli Zaretskii wrote:
> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> >> Date: Tue, 22 Oct 2024 06:46:15 +0200
> >> 
> >> I have (re-)created the scratch/tty-child-frames branch today, which
> >> contains the code for child frames on ttys, based on a recent master.
> > 
> > Thanks.
> > 
> >> Disclaimer: As I mentioned already in other contexts, I don't want to
> >> be the maintainer of anything, for personal reasons.
> > 
> > We have difficulty working with "fire and forget" contributions of
> > this size and complexity.  Maybe Jared (CC'ed) could take a look and
> > clean up the branch, to prepare it for landing.  Or someone else with
> > interest in this stuff (Martin?).
> 
> I'm happy to be considered here and would be interested in helping out, 
> but it would have to wait. I'm extremely busy at my job for the next 
> month or so and unfortunately do not have time to take a contribution 
> such as this to completion.
> 
> I'll check back in when my time clears up. I would certainly be 
> interested in helping here if needed. I am certainly interested in 
> making TTY Emacs more functional in general.

Jared, if you have time now, please take a look at this branch.  From
where I stand, once it works well with xt-mouse, we can land this
feature on master.

TIA

Re: "Final" version of tty child frames

[Jared Finder]

On 2024-11-30 03:25, Eli Zaretskii wrote:
>> Date: Sun, 27 Oct 2024 21:35:29 -0700
>> From: Jared Finder <jared@finder.org>
>> Cc: Gerd Möllmann <gerd.moellmann@gmail.com>,
>>  emacs-devel@gnu.org, martin rudalics <rudalics@gmx.at>
>> 
>> On 2024-10-21 22:29, Eli Zaretskii wrote:
>> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> >> Date: Tue, 22 Oct 2024 06:46:15 +0200
>> >>
>> >> I have (re-)created the scratch/tty-child-frames branch today, which
>> >> contains the code for child frames on ttys, based on a recent master.
>> >
>> > Thanks.
>> >
>> >> Disclaimer: As I mentioned already in other contexts, I don't want to
>> >> be the maintainer of anything, for personal reasons.
>> >
>> > We have difficulty working with "fire and forget" contributions of
>> > this size and complexity.  Maybe Jared (CC'ed) could take a look and
>> > clean up the branch, to prepare it for landing.  Or someone else with
>> > interest in this stuff (Martin?).
>> 
>> I'm happy to be considered here and would be interested in helping 
>> out,
>> but it would have to wait. I'm extremely busy at my job for the next
>> month or so and unfortunately do not have time to take a contribution
>> such as this to completion.
>> 
>> I'll check back in when my time clears up. I would certainly be
>> interested in helping here if needed. I am certainly interested in
>> making TTY Emacs more functional in general.
> 
> Jared, if you have time now, please take a look at this branch.  From
> where I stand, once it works well with xt-mouse, we can land this
> feature on master.

Yup, I'll start looking into this starting this weekend.

   -- MJF

Re: "Final" version of tty child frames

[Jared Finder]

On 2024-12-04 19:49, Jared Finder wrote:
> On 2024-11-30 03:25, Eli Zaretskii wrote:
>>> Date: Sun, 27 Oct 2024 21:35:29 -0700
>>> From: Jared Finder <jared@finder.org>
>>> Cc: Gerd Möllmann <gerd.moellmann@gmail.com>,
>>>  emacs-devel@gnu.org, martin rudalics <rudalics@gmx.at>
>>> 
>>> On 2024-10-21 22:29, Eli Zaretskii wrote:
>>> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>> >> Date: Tue, 22 Oct 2024 06:46:15 +0200
>>> >>
>>> >> I have (re-)created the scratch/tty-child-frames branch today, which
>>> >> contains the code for child frames on ttys, based on a recent master.
>>> >
>>> > Thanks.
>>> >
>>> >> Disclaimer: As I mentioned already in other contexts, I don't want to
>>> >> be the maintainer of anything, for personal reasons.
>>> >
>>> > We have difficulty working with "fire and forget" contributions of
>>> > this size and complexity.  Maybe Jared (CC'ed) could take a look and
>>> > clean up the branch, to prepare it for landing.  Or someone else with
>>> > interest in this stuff (Martin?).
>>> 
>>> I'm happy to be considered here and would be interested in helping 
>>> out,
>>> but it would have to wait. I'm extremely busy at my job for the next
>>> month or so and unfortunately do not have time to take a contribution
>>> such as this to completion.
>>> 
>>> I'll check back in when my time clears up. I would certainly be
>>> interested in helping here if needed. I am certainly interested in
>>> making TTY Emacs more functional in general.
>> 
>> Jared, if you have time now, please take a look at this branch.  From
>> where I stand, once it works well with xt-mouse, we can land this
>> feature on master.
> 
> Yup, I'll start looking into this starting this weekend.

Spent time just playing with child frames to get familiar with things. A 
few bugs I noticed:

There's something in the C code that makes this branch not work with the 
mini-frame package (https://github.com/muffinmad/emacs-mini-frame). The 
package works fine in graphical mode.

I'm using posframe for iteration. There's a minor thing where the cursor 
position is inconsistent when at the end of a line with a posframe on it 
in the tty. Maybe that's intentional? It's kinda confusing to me.

There's also a major thing where I can click on the child frame in the 
tty and then my cursor is actually in the child frame and I can just 
alter the text there. That behavior is inconsistent with the behavior on 
graphical displays and something I can investigate.

   -- MJF

Re: "Final" version of tty child frames

[Gerd Möllmann]

Jared Finder <jared@finder.org> writes:

> Spent time just playing with child frames to get familiar with things.
> A few bugs I noticed:

Nice!

> There's something in the C code that makes this branch not work with
> the mini-frame package
> (https://github.com/muffinmad/emacs-mini-frame). The package works
> fine in graphical mode.

The page says that the package is using mini-buffer-only frames, which
are not implemented, because I wasn't interested. See the #if 0
make_terminal_frame.

> I'm using posframe for iteration. There's a minor thing where the
> cursor position is inconsistent when at the end of a line with a
> posframe on it in the tty. Maybe that's intentional? It's kinda
> confusing to me.

Could you please describe in detail what you are doing? I haven't
understood yet. Posframe and Corfu are BTW the reason I added the
child frames.

Child frames in the more general sense was not so interesting. Always
talking about me personally of course. I think I talked with Martin
about adding stuff as needed if users really want it, or something, and
ISTR he agreed. But Martin can speak for himself :-).

> There's also a major thing where I can click on the child frame in the
> tty and then my cursor is actually in the child frame and I can just
> alter the text there. That behavior is inconsistent with the behavior
> on graphical displays and something I can investigate.

Could you please give a recipe? I don't understand yet what you mean.
Or, if you want to investigate, please do :-). Please don't hesitate to
ask me any question. At least I still remember more stuff about this
than about igc :-).

Re: "Final" version of tty child frames

[Jared Finder]

On 2024-12-11 02:59, Gerd Möllmann wrote:
> Jared Finder <jared@finder.org> writes:
> 
>> Spent time just playing with child frames to get familiar with things.
>> A few bugs I noticed:
> 
> Nice!
> 
>> There's something in the C code that makes this branch not work with
>> the mini-frame package
>> (https://github.com/muffinmad/emacs-mini-frame). The package works
>> fine in graphical mode.
> 
> The page says that the package is using mini-buffer-only frames, which
> are not implemented, because I wasn't interested. See the #if 0
> make_terminal_frame.

I think it would be good for unimplemented features to communicate that 
state to the user so users know clearly what is going on.  Right now the 
error a user sees is "Can’t change the ‘minibuffer’ parameter of this 
frame". Wouldn't it be better to have make-terminal-frame (a brand new 
function with no existing clients to support) error with something like 
"Minibuffer only frames are not supported in terminals"?

This also would avoid the bug that currently a minibuffer-only child 
frame gets left around which looks weird and confusing.

Are there any other intentionally unimplemented features?  It'd be nice 
to have similarly clean errors to inform users.

>> I'm using posframe for iteration. There's a minor thing where the
>> cursor position is inconsistent when at the end of a line with a
>> posframe on it in the tty. Maybe that's intentional? It's kinda
>> confusing to me.
> 
> Could you please describe in detail what you are doing? I haven't
> understood yet. Posframe and Corfu are BTW the reason I added the
> child frames.
> 
> Child frames in the more general sense was not so interesting. Always
> talking about me personally of course. I think I talked with Martin
> about adding stuff as needed if users really want it, or something, and
> ISTR he agreed. But Martin can speak for himself :-).

This was from me moving the cursor around (just using arrow keys) the 
text covered up by a child frame.

>> There's also a major thing where I can click on the child frame in the
>> tty and then my cursor is actually in the child frame and I can just
>> alter the text there. That behavior is inconsistent with the behavior
>> on graphical displays and something I can investigate.
> 
> Could you please give a recipe? I don't understand yet what you mean.
> Or, if you want to investigate, please do :-). Please don't hesitate to
> ask me any question. At least I still remember more stuff about this
> than about igc :-).

This is from me using the mouse while in xterm and clicking on the child 
frame.  Or just hovering over the child frame with 
mouse-autoselect-window set to t.  I'm guessing this is just due to the 
frame parameter no-accept-focus not being implemented.  Since this is a 
common path for posframe, I think it could be important to implement. 
Such an implementation probably needs to alter xt-mouse.el's generation 
of select-window events.

To reproduce:

(xterm-mouse-mode 1)
(posframe-show "*scratch*" :poshandler 
'posframe-poshandler-frame-center)
Click on the posframe.

   -- MJF

Re: "Final" version of tty child frames

[Gerd Möllmann]

Jared Finder <jared@finder.org> writes:

> On 2024-12-11 02:59, Gerd Möllmann wrote:
>> Jared Finder <jared@finder.org> writes:
>> 
>>> Spent time just playing with child frames to get familiar with things.
>>> A few bugs I noticed:
>> Nice!
>> 
>>> There's something in the C code that makes this branch not work with
>>> the mini-frame package
>>> (https://github.com/muffinmad/emacs-mini-frame). The package works
>>> fine in graphical mode.
>> The page says that the package is using mini-buffer-only frames,
>> which
>> are not implemented, because I wasn't interested. See the #if 0
>> make_terminal_frame.
>
> I think it would be good for unimplemented features to communicate
> that state to the user so users know clearly what is going on.  Right
> now the error a user sees is "Can’t change the ‘minibuffer’ parameter
> of this frame". Wouldn't it be better to have make-terminal-frame (a
> brand new function with no existing clients to support) error with
> something like "Minibuffer only frames are not supported in
> terminals"?
>
> This also would avoid the bug that currently a minibuffer-only child
> frame gets left around which looks weird and confusing.

Sure, we could do that, or someone implements minibuffer-only frames,
someone knowing how these work and are used on GUI. Or IOW, I feel
things are still a bit in undecided state, except on my side.

> Are there any other intentionally unimplemented features?  It'd be
> nice to have similarly clean errors to inform users.

Open things should all be marked them all with FIXME/tty. Frame
re-stacking (tty-frame-restack) is an example, or specifying frame
borders (what a WM would do).

>>> I'm using posframe for iteration. There's a minor thing where the
>>> cursor position is inconsistent when at the end of a line with a
>>> posframe on it in the tty. Maybe that's intentional? It's kinda
>>> confusing to me.
>> Could you please describe in detail what you are doing? I haven't
>> understood yet. Posframe and Corfu are BTW the reason I added the
>> child frames.
>> Child frames in the more general sense was not so interesting.
>> Always
>> talking about me personally of course. I think I talked with Martin
>> about adding stuff as needed if users really want it, or something, and
>> ISTR he agreed. But Martin can speak for himself :-).
>
> This was from me moving the cursor around (just using arrow keys) the
> text covered up by a child frame.

So you have a child frame open covering a line of text in the parent
frame. And then move the cursor in the parent through obscured line? And
what do you mean with the cursor position being inconsistent?

(That would BTW be a child frame use-case which is not so interesting
for me personally, and something GNU has to decide if it's
important/essential or whatever. When is write GNU I mean whoever
decides that at the end.)

>>> There's also a major thing where I can click on the child frame in the
>>> tty and then my cursor is actually in the child frame and I can just
>>> alter the text there. That behavior is inconsistent with the behavior
>>> on graphical displays and something I can investigate.
>> Could you please give a recipe? I don't understand yet what you
>> mean.
>> Or, if you want to investigate, please do :-). Please don't hesitate to
>> ask me any question. At least I still remember more stuff about this
>> than about igc :-).
>
> This is from me using the mouse while in xterm and clicking on the
> child frame.  Or just hovering over the child frame with
> mouse-autoselect-window set to t.  I'm guessing this is just due to
> the frame parameter no-accept-focus not being implemented.  Since this
> is a common path for posframe, I think it could be important to
> implement. Such an implementation probably needs to alter
> xt-mouse.el's generation of select-window events.
>
> To reproduce:
>
> (xterm-mouse-mode 1)
> (posframe-show "*scratch*" :poshandler
> 'posframe-poshandler-frame-center)
> Click on the posframe.
>
>   -- MJF

Could be no-accept-focus, I don't know. If so, and IIRC how that words,
this is another case where not having a window manager comes into play,
and one would have to decide If one wants to implement part of the
functionality of a WM in Emacs on ttys.

(One WM feature I've added because I think one cannot do without is
frame decoration/border. With the caveat that I could not get things to
work by using border-width and so on, with the result that git reset
--hard in a rage in the end (which doesn't happen often), and I don't
think I'll try again soon :-))

Re: "Final" version of tty child frames

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

>> Are there any other intentionally unimplemented features?  It'd be
>> nice to have similarly clean errors to inform users.

Forgot something:

"Real" tooltip frames are not implemented. At the time I added toolips,
Emacs didn't have child frames, which led to the horrible special
handling of tooltips frames for GUIs.

And the tooltip code is spread and repeated over different GUI source
files.

I couldn't be bothered to untangle this to add tooltips for ttys, 
but there is tty-tip.el which does something very close using child
frames. Just use tty-tip-mode and move the mouse over mode-line items,
for example. Or over flymake errors, and so on.

Re: "Final" version of tty child frames

[Eli Zaretskii]

> Date: Thu, 12 Dec 2024 00:11:01 -0500
> From: Jared Finder <jared@finder.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org, rudalics@gmx.at
> 
> I think it would be good for unimplemented features to communicate that 
> state to the user so users know clearly what is going on.  Right now the 
> error a user sees is "Can’t change the ‘minibuffer’ parameter of this 
> frame". Wouldn't it be better to have make-terminal-frame (a brand new 
> function with no existing clients to support) error with something like 
> "Minibuffer only frames are not supported in terminals"?

I think minibuffer-only frames _are_ implemented on TTYs (albeit not
very useful there).  They are not implemented as child frames, I
think.

But yes, emitting an explicit error message about something not
implemented would be definitely better.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Thu, 12 Dec 2024 00:11:01 -0500
>> From: Jared Finder <jared@finder.org>
>> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org, rudalics@gmx.at
>> 
>> I think it would be good for unimplemented features to communicate that 
>> state to the user so users know clearly what is going on.  Right now the 
>> error a user sees is "Can’t change the ‘minibuffer’ parameter of this 
>> frame". Wouldn't it be better to have make-terminal-frame (a brand new 
>> function with no existing clients to support) error with something like 
>> "Minibuffer only frames are not supported in terminals"?
>
> I think minibuffer-only frames _are_ implemented on TTYs (albeit not
> very useful there).  They are not implemented as child frames, I
> think.
>
> But yes, emitting an explicit error message about something not
> implemented would be definitely better.

Pushed something like that to the branch.

Re: "Final" version of tty child frames

[martin rudalics]

 > There's something in the C code that makes this branch not work with
 > the mini-frame package
 > (https://github.com/muffinmad/emacs-mini-frame). The package works
 > fine in graphical mode.

IIUC I use something similar to the mini-frame package and it's already
quite hairy to make the minibuffer frame pop up and automatically get
input focus.  The input focus thing will be hard to implement on ttys.

 > I'm using posframe for iteration. There's a minor thing where the
 > cursor position is inconsistent when at the end of a line with a
 > posframe on it in the tty. Maybe that's intentional? It's kinda
 > confusing to me.

Is the cursor in the normal window or in the posframe window?

 > There's also a major thing where I can click on the child frame in the
 > tty and then my cursor is actually in the child frame and I can just
 > alter the text there. That behavior is inconsistent with the behavior
 > on graphical displays and something I can investigate.

Do you mean that you do _not_ want to alter text in the child frame?

If you have the time, please also try to look into two issues I raised
earlier:

   Two further things I noticed: When point in the parent frame is
   effectively hidden by the child frame, its cursor sometimes appears at
   the right of the child frame and sometimes it's not shown.  I have not
   understood the underlying principle for this behavior.  Also when the
   selected region in the parent frame is active, its overlay covers the
   child frame.  That's ugly.

martin

Re: "Final" version of tty child frames

[Dr. Arne Babenhauserheide]

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

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> - Documentation. Would only make sense to write if this goes into GNU,
>   which might or might not happen, depending on, from my side, how
>   much work that is.

Do you already have copyright assignment done so people can simply take
this up and merge it where it fits?

Best wishes,
Arne
-- 
Unpolitisch sein
heißt politisch sein,
ohne es zu merken.
draketo.de

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 1125 bytes --]

Re: "Final" version of tty child frames

[Gerd Möllmann]

"Dr. Arne Babenhauserheide" <arne_bab@web.de> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> - Documentation. Would only make sense to write if this goes into GNU,
>>   which might or might not happen, depending on, from my side, how
>>   much work that is.
>
> Do you already have copyright assignment done so people can simply take
> this up and merge it where it fits?

I have :-).

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: "Dr. Arne Babenhauserheide" <arne_bab@web.de>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Date: Tue, 22 Oct 2024 09:34:55 +0200
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > - Documentation. Would only make sense to write if this goes into GNU,
> >   which might or might not happen, depending on, from my side, how
> >   much work that is.
> 
> Do you already have copyright assignment done so people can simply take
> this up and merge it where it fits?

Gerd has assignment on file for a long time.  Otherwise, he wouldn't
have had write access to the Emacs Git repository, and couldn't have
pushed this branch in the first place.

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Tue, 22 Oct 2024 06:46:15 +0200
> 
> I have (re-)created the scratch/tty-child-frames branch today, which
> contains the code for child frames on ttys, based on a recent master.
> 
> I'm a happy user of this for a while now with corfu, vertico +
> vertico-posframe + consult, transient + transient-posframe,
> which-key + which-key-posframe. And my current todo list is now empty,
> so here it is.

Is there a way to test the feature without using corfu?  If so, can
you suggest some simple Lisp to see if the child frames work in a
build of this branch?

> Disclaimer: As I mentioned already in other contexts, I don't want to
> be the maintainer of anything, for personal reasons.
> 
> Have fun!

Does this compile cleanly for you?  I get gobs of warnings like this:

  dispnew.c: In function ‘gui_update_window_end’:
  dispnew.c:4495:34: warning: potential null pointer dereference [-Wnull-dereference]
   4495 |       hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
	|       ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This seems to be because you've changed the definition of
MOUSE_HL_INFO to be this:

  #   define MOUSE_HL_INFO(F)                                     \
    (FRAME_WINDOW_P (F)                                           \
    ? (FRAME_OUTPUT_DATA (F)                                      \
       ? &FRAME_DISPLAY_INFO (F)->mouse_highlight                 \
       : NULL)                                                    \
     : &(F)->output_data.tty->display_info->mouse_highlight)

I don't understand the need for this NULL there.  What is its purpose,
and what will we lose by going back to the original definition?  I
believe that NULL is what's causing these warnings.

Thanks.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Date: Tue, 22 Oct 2024 06:46:15 +0200
>> 
>> I have (re-)created the scratch/tty-child-frames branch today, which
>> contains the code for child frames on ttys, based on a recent master.
>> 
>> I'm a happy user of this for a while now with corfu, vertico +
>> vertico-posframe + consult, transient + transient-posframe,
>> which-key + which-key-posframe. And my current todo list is now empty,
>> so here it is.
>
> Is there a way to test the feature without using corfu?  If so, can
> you suggest some simple Lisp to see if the child frames work in a
> build of this branch?

Thanks for taking a look.

Something built-in you could try is show-paren-mode with
show-paren-context-when-offscreen set to 'child-frame. Not very useful,
but it should display a child frame at (0,0) of a window.

Or maybe

(defun my-make-child ()
  (interactive)
  (make-frame `((parent-frame . ,(selected-frame))
		(background-color . "gray10")
		(foreground-color . "white")
		(internal-border-width . 1)
		(top . 15)
		(left . 40)
		(width . 80)
		(height . 25))))

which is not really the use case I have in mind but anyway.

>> Disclaimer: As I mentioned already in other contexts, I don't want to
>> be the maintainer of anything, for personal reasons.
>> 
>> Have fun!
>
> Does this compile cleanly for you?  I get gobs of warnings like this:

Yes it does, with clang.

>   dispnew.c: In function ‘gui_update_window_end’:
>   dispnew.c:4495:34: warning: potential null pointer dereference [-Wnull-dereference]
>    4495 |       hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
> 	|       ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This seems to be because you've changed the definition of
> MOUSE_HL_INFO to be this:
>
>   #   define MOUSE_HL_INFO(F)                                     \
>     (FRAME_WINDOW_P (F)                                           \
>     ? (FRAME_OUTPUT_DATA (F)                                      \
>        ? &FRAME_DISPLAY_INFO (F)->mouse_highlight                 \
>        : NULL)                                                    \
>      : &(F)->output_data.tty->display_info->mouse_highlight)
>
> I don't understand the need for this NULL there.  What is its purpose,
> and what will we lose by going back to the original definition?  I
> believe that NULL is what's causing these warnings.

It probably crept in when I ported this. I can remove that if you want.

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 22 Oct 2024 10:21:43 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> (defun my-make-child ()
>   (interactive)
>   (make-frame `((parent-frame . ,(selected-frame))
> 		(background-color . "gray10")
> 		(foreground-color . "white")
> 		(internal-border-width . 1)
> 		(top . 15)
> 		(left . 40)
> 		(width . 80)
> 		(height . 25))))
> 
> which is not really the use case I have in mind but anyway.

Thanks.

> > Does this compile cleanly for you?  I get gobs of warnings like this:
> 
> Yes it does, with clang.
> 
> >   dispnew.c: In function ‘gui_update_window_end’:
> >   dispnew.c:4495:34: warning: potential null pointer dereference [-Wnull-dereference]
> >    4495 |       hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
> > 	|       ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > This seems to be because you've changed the definition of
> > MOUSE_HL_INFO to be this:
> >
> >   #   define MOUSE_HL_INFO(F)                                     \
> >     (FRAME_WINDOW_P (F)                                           \
> >     ? (FRAME_OUTPUT_DATA (F)                                      \
> >        ? &FRAME_DISPLAY_INFO (F)->mouse_highlight                 \
> >        : NULL)                                                    \
> >      : &(F)->output_data.tty->display_info->mouse_highlight)
> >
> > I don't understand the need for this NULL there.  What is its purpose,
> > and what will we lose by going back to the original definition?  I
> > believe that NULL is what's causing these warnings.
> 
> It probably crept in when I ported this. I can remove that if you want.

No need, already done.

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 22 Oct 2024 10:21:43 +0200
> 
> Thanks for taking a look.

Can you explain this comment and the code change:

  +         /* Struct frame can move with igc, and so on.  But we need
  +            something that takes different frames into account. Use the
  +            face_cache pointer for that which is malloc'd. */
  +         if (glyph->frame && glyph->frame != f)
  +           face_id += (ptrdiff_t) glyph->frame->face_cache;

Why do we need to take the frame into account here?  Is this relevant
to the current GC used on master?

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 22 Oct 2024 10:21:43 +0200
>> 
>> Thanks for taking a look.
>
> Can you explain this comment and the code change:
>
>   +         /* Struct frame can move with igc, and so on.  But we need
>   +            something that takes different frames into account. Use the
>   +            face_cache pointer for that which is malloc'd. */
>   +         if (glyph->frame && glyph->frame != f)
>   +           face_id += (ptrdiff_t) glyph->frame->face_cache;
>
> Why do we need to take the frame into account here?  Is this relevant
> to the current GC used on master?

I can try.

A combined frame matrix's glyph contents may come from different frames,
either the root frame one of its descendants (children, grandchildren
and so on). See copy_child_glyphs. glyph->frame is th eframe from where
the glyph stems. It is filled out when producing glyphs.

Face ids are valid only in frame's face cache. A Face with a given id in
one frame may be different from the same id another frame. So that's why
I'm taking the face cache into account here. Otherwise it could happen
that row hashes are the same although the contents are different. Not a
big deal, but one can avoid it, so I did.

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 22 Oct 2024 12:23:04 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> >> Cc: emacs-devel@gnu.org
> >> Date: Tue, 22 Oct 2024 10:21:43 +0200
> >> 
> >> Thanks for taking a look.
> >
> > Can you explain this comment and the code change:
> >
> >   +         /* Struct frame can move with igc, and so on.  But we need
> >   +            something that takes different frames into account. Use the
> >   +            face_cache pointer for that which is malloc'd. */
> >   +         if (glyph->frame && glyph->frame != f)
> >   +           face_id += (ptrdiff_t) glyph->frame->face_cache;
> >
> > Why do we need to take the frame into account here?  Is this relevant
> > to the current GC used on master?
> 
> I can try.
> 
> A combined frame matrix's glyph contents may come from different frames,
> either the root frame one of its descendants (children, grandchildren
> and so on). See copy_child_glyphs. glyph->frame is th eframe from where
> the glyph stems. It is filled out when producing glyphs.
> 
> Face ids are valid only in frame's face cache. A Face with a given id in
> one frame may be different from the same id another frame. So that's why
> I'm taking the face cache into account here. Otherwise it could happen
> that row hashes are the same although the contents are different. Not a
> big deal, but one can avoid it, so I did.

Why not simply include the frame pointer in the hash?  Adding a
pointer to a number looks kludgey, and might even make the hash
weaker.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 22 Oct 2024 12:23:04 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> >> Cc: emacs-devel@gnu.org
>> >> Date: Tue, 22 Oct 2024 10:21:43 +0200
>> >> 
>> >> Thanks for taking a look.
>> >
>> > Can you explain this comment and the code change:
>> >
>> >   +         /* Struct frame can move with igc, and so on.  But we need
>> >   +            something that takes different frames into account. Use the
>> >   +            face_cache pointer for that which is malloc'd. */
>> >   +         if (glyph->frame && glyph->frame != f)
>> >   +           face_id += (ptrdiff_t) glyph->frame->face_cache;
>> >
>> > Why do we need to take the frame into account here?  Is this relevant
>> > to the current GC used on master?
>> 
>> I can try.
>> 
>> A combined frame matrix's glyph contents may come from different frames,
>> either the root frame one of its descendants (children, grandchildren
>> and so on). See copy_child_glyphs. glyph->frame is th eframe from where
>> the glyph stems. It is filled out when producing glyphs.
>> 
>> Face ids are valid only in frame's face cache. A Face with a given id in
>> one frame may be different from the same id another frame. So that's why
>> I'm taking the face cache into account here. Otherwise it could happen
>> that row hashes are the same although the contents are different. Not a
>> big deal, but one can avoid it, so I did.
>
> Why not simply include the frame pointer in the hash?  

With igc, frames move in memory, face caches are malloc'd.

> Adding a pointer to a number looks kludgey, and might even make the
> hash weaker.

If you have something better, please tell. 

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 22 Oct 2024 15:43:29 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Adding a pointer to a number looks kludgey, and might even make the
> > hash weaker.
> 
> If you have something better, please tell. 

Give each frame a number, and use that in the hash.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 22 Oct 2024 15:43:29 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Adding a pointer to a number looks kludgey, and might even make the
>> > hash weaker.
>> 
>> If you have something better, please tell. 
>
> Give each frame a number, and use that in the hash.

Thanks, that would work of course. Don't know if it's worth the effort.

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 22 Oct 2024 16:02:39 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> >> Cc: emacs-devel@gnu.org
> >> Date: Tue, 22 Oct 2024 15:43:29 +0200
> >> 
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> > Adding a pointer to a number looks kludgey, and might even make the
> >> > hash weaker.
> >> 
> >> If you have something better, please tell. 
> >
> > Give each frame a number, and use that in the hash.
> 
> Thanks, that would work of course. Don't know if it's worth the effort.

Btw, the current code has a problem here:

          int face_id = glyph->face_id;
	  [...]
          if (glyph->frame && glyph->frame != f)
            face_id += (ptrdiff_t) glyph->frame->face_cache;
	  [...]
          hash = (((hash << 4) + (hash >> 24)) & 0x0fffffff) + c;
          hash = (((hash << 4) + (hash >> 24)) & 0x0fffffff) + face_id;

face_id is an 'int', but 'ptrdiff_t' is a 64-bit data type in 64-bit
builds.  So the addition of face_cache pointer could overflow, which
AFAIK is UB with signed data types.  Moreover, 'hash' is declared
'unsigned int', and so is the data type returned by line_hash_code,
which only makes the problem worse.

Paul, should we make line_hash_code return 'size_t' instead (and
change the data type of the variables in 'scrolling' accordingly)?  Or
maybe we should simply mask off high bits of the face_cache's pointer,
leaving only the low 32 bits, before adding it to the hash?

Re: "Final" version of tty child frames

[Paul Eggert]

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

On 2024-10-22 07:40, Eli Zaretskii wrote:
> Paul, should we make line_hash_code return 'size_t' instead (and
> change the data type of the variables in 'scrolling' accordingly)?  Or
> maybe we should simply mask off high bits of the face_cache's pointer,
> leaving only the low 32 bits, before adding it to the hash?

Either approach should be correct. The attached (untested and 
uninstalled) patch does the equivalent of the latter, which is simpler. 
I don't know whether the more-complicated one would perform better.

This patch also fixes a glitch in that one should cast pointers to 
uintptr_t or to intptr_t, not to ptrdiff_t. The difference can matter on 
unusual platforms like CheriBSD where uintptr_t is wider than ptrdiff_t.

[-- Attachment #2: 0001-Fix-UB-in-line_hash_code.patch --]
[-- Type: text/x-patch, Size: 1157 bytes --]

From f624661eec854f09b28076b8f1bf80b1ed326475 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 22 Oct 2024 12:18:14 -0700
Subject: [PATCH] Fix UB in line_hash_code

* src/dispnew.c (line_hash_code): Avoid undefined behavior on
integer overflow.
---
 src/dispnew.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index 200ffaaca21..1ece9cc1d45 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -1175,12 +1175,12 @@ line_hash_code (struct frame *f, struct glyph_row *row)
       while (glyph < end)
 	{
 	  int c = glyph->u.ch;
-	  int face_id = glyph->face_id;
+	  unsigned int face_id = glyph->face_id;
 	  /* Struct frame can move with igc, and so on.  But we need
 	     something that takes different frames into account.  Use the
 	     face_cache pointer for that which is malloc'd.  */
 	  if (glyph->frame && glyph->frame != f)
-	    face_id += (ptrdiff_t) glyph->frame->face_cache;
+	    face_id += (uintptr_t) glyph->frame->face_cache;
 	  if (FRAME_MUST_WRITE_SPACES (f))
 	    c -= SPACEGLYPH;
 	  hash = (((hash << 4) + (hash >> 24)) & 0x0fffffff) + c;
-- 
2.43.0

Re: "Final" version of tty child frames

[Gerd Möllmann]

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 2024-10-22 07:40, Eli Zaretskii wrote:
>> Paul, should we make line_hash_code return 'size_t' instead (and
>> change the data type of the variables in 'scrolling' accordingly)?  Or
>> maybe we should simply mask off high bits of the face_cache's pointer,
>> leaving only the low 32 bits, before adding it to the hash?
>
> Either approach should be correct. The attached (untested and
> uninstalled) patch does the equivalent of the latter, which is
> simpler. I don't know whether the more-complicated one would perform
> better.
>
> This patch also fixes a glitch in that one should cast pointers to
> uintptr_t or to intptr_t, not to ptrdiff_t. The difference can matter
> on unusual platforms like CheriBSD where uintptr_t is wider than
> ptrdiff_t.

Thanks Paul, I've pushed that to the branch.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org
>> Date: Tue, 22 Oct 2024 10:21:43 +0200
>> 
>> Thanks for taking a look.
>
> Can you explain this comment and the code change:
>
>   +         /* Struct frame can move with igc, and so on.  But we need
>   +            something that takes different frames into account. Use the
>   +            face_cache pointer for that which is malloc'd. */
>   +         if (glyph->frame && glyph->frame != f)
>   +           face_id += (ptrdiff_t) glyph->frame->face_cache;
>
> Why do we need to take the frame into account here?  Is this relevant
> to the current GC used on master?

Forgot to answer tha GC part: It's not relevant to non-MPS GC. (I'm
using igc here, normally.)

Re: "Final" version of tty child frames

[Feng Shu]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

Will the below interfaces changed?

1. tty-child-frames
2. tty-non-selected-cursor
3. undecorated

If not, I have updated posframe, vertico-posframe and ivy-posframe to
support three interfaces.


> I have (re-)created the scratch/tty-child-frames branch today, which
> contains the code for child frames on ttys, based on a recent master.
>
> I'm a happy user of this for a while now with corfu, vertico +
> vertico-posframe + consult, transient + transient-posframe,
> which-key + which-key-posframe. And my current todo list is now empty,
> so here it is.
>
> To use it, redefine these two functions:
>
> #+begin_src emacs-lisp
> (defun posframe-workable-p ()
>   "Test posframe workable status."
>   (and (>= emacs-major-version 26)
>        (not (or noninteractive
>                 emacs-basic-display
>                 (or (featurep 'tty-child-frames)
>                     (not (display-graphic-p)))
>                 (eq (frame-parameter (selected-frame) 'minibuffer) 'only)))))))
>
> (cl-defgeneric corfu--popup-support-p ()
>   "Return non-nil if child frames are supported."
>   (or (display-graphic-p)
>       (featurep 'tty-child-frames)))
> #+end_src
>
>
> To make things look nicer you might also want to
>
> #+begin_src emacs-lisp
> (push '(tty-non-selected-cursor . t) vertico-posframe-parameters)
> (push '(undecorated . nil) vertico-posframe-parameters))
> (push '(undecorated . nil) transient-posframe-parameters))
> #+end_src
>
> The 'undecorated' frame parameter lets Emacs draw a border around the
> child frame (default is no border). The tty-non-selected-cursor
> parameter makes redisplay put the terminal cursor in a non-selected
> frame which I find nice for things like consult-buffer. Which is why I
> added it :-).
>
> What's there fits my personal needs entirely. I am not interested in
> full support of child frames as they exist on window systems because I
> don't see child frames being used except for things like posframe and
> corfu and similar. And, TBH, I don't find the tty frame code fun to
> work with.
>
> Other things not contained:
>
> - Support for anything but termcap frames. I bet I broke MSDOS.
> - Mouse support except with xterm-mouse-mode (no GPM).
> - Drawing borders like it is normally done (internal border and so
>   on). Tried that once, git reset --hard, won't happen.
> - Tooltips. I think tooltips could relatively easily be implemented
>   with child frames by copying or extracting the non-native tooltip
>   code from x-show-tip or some such. From my POV, that would be best
>   done by refactoring the tooltip code across window systems which I
>   can't do.
> - Documentation. Would only make sense to write if this goes into GNU,
>   which might or might not happen, depending on, from my side, how
>   much work that is.
>
> Disclaimer: As I mentioned already in other contexts, I don't want to
> be the maintainer of anything, for personal reasons.
>
> Have fun!
>
>

-- 

Re: "Final" version of tty child frames

[Gerd Möllmann]

Feng Shu <tumashu@163.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
> Will the below interfaces changed?
>
> 1. tty-child-frames
> 2. tty-non-selected-cursor
> 3. undecorated

I don't plan to change anything.

>
> If not, I have updated posframe, vertico-posframe and ivy-posframe to
> support three interfaces.

Very nice, thank you!

I also got a message from github that Daniel added support to Corfu.

Re: "Final" version of tty child frames

[Feng Shu]

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

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Feng Shu <tumashu@163.com> writes:
>
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>
>> Will the below interfaces changed?
>>
>> 1. tty-child-frames
>> 2. tty-non-selected-cursor
>> 3. undecorated

border do not work well when current buffer is CJK, maybe because CJK Char width is
not 1



[-- Attachment #2: 2024-10-23_11-22.png --]
[-- Type: image/png, Size: 108954 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]




>
> I don't plan to change anything.
>
>>
>> If not, I have updated posframe, vertico-posframe and ivy-posframe to
>> support three interfaces.
>
> Very nice, thank you!
>
> I also got a message from github that Daniel added support to Corfu.

-- 

Re: "Final" version of tty child frames

[Gerd Möllmann]

Feng Shu <tumashu@163.com> writes:

> border do not work well when current buffer is CJK, maybe because CJK
> Char width is not 1

Could you please send me a bit of CJK text so that I have something I
can test this with?

Re: "Final" version of tty child frames

[Feng Shu]

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

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Feng Shu <tumashu@163.com> writes:
>
>> border do not work well when current buffer is CJK, maybe because CJK
>> Char width is not 1
>
> Could you please send me a bit of CJK text so that I have something I
> can test this with?


[-- Attachment #2: 1.org --]
[-- Type: text/plain, Size: 4183 bytes --]

* 《红楼梦》的版本

曹雪芹的《红楼梦》亲笔原稿肯定早就找不到了，是不是因为原稿字迹太潦草，让人抄齐了以后他就不想再留，烧了呢？因此现存所有的本子都是抄的，而且还都不是直接从曹雪芹原稿手迹抄，是二传手，是抄本的抄本，甚至是三传手，四传手。大家抄来抄去，很多抄手还特别有才，自由发挥添加删改了不少地方。这些不同的版本有很多，所以自然就产生了这样一个问题：哪一个版本是最接近曹雪芹原著的？看《红楼梦》应该看哪一种最好？所以有必要把这些版本都列一下：
* 带脂评的手抄本，共11个版本：


书名《脂砚斋重评石头记》，胡适在1927年发现的，现藏于美国康奈尔大学。只有1—8，13—16，25—28，共16回，4册，4回一册，有一千多条脂批。第一回有其它各本没有的一句话：“至脂砚斋甲戌抄阅再评仍用石头记”所以叫甲戌本。只有此本有“凡例”，后两本把凡例融合在了正文里。甲戌是1754年，乾隆19年，红楼梦刚写完，曹雪芹39岁。
* 这是第一个基本成熟的本子，虽然残缺很多，但原本肯定是完整的，胡适的这个甲戌本当然不是1754年的原抄本，是谁在什么时候抄的已经无法考证，应该晚于程本的1791年。1754年之前的本子一般认为是不成熟本，缺失太多，分散出现在庚辰本之后的本子里，包括程本。

2. 己卯本：


3. 庚辰本：

书名《脂砚斋重评石头记》，是在己卯本基础上修改后的最后定本，因此是所有版本中最重要的本子，可能最接近曹雪芹原笔的。1933年由徐星署所得，现存北大图书馆。此本只缺64，67回，当时抄的时候底本就少了这两回。因后四册目录页有“庚辰秋月定本”，所以叫庚辰本。庚辰是1760年，曹雪芹45岁。现存的这个本子当然不是1760年抄录的，但具体什么时间，什么人抄的已经没法考证了。
* 只有甲戌己卯庚辰这老三篇来源单一，血统比较纯正，此后的版本大都是各种本子的混合，来源混杂。曹雪芹原稿的书名显然是《石头记》而不是《红楼梦》。

4. 梦稿本：

书名《乾隆抄本百廿回红楼梦稿》，杨继振藏本，杨藏本，现存国博。大部分抄自甲戌己卯庚辰。其中67回是唯一来源于当时没有缺失的老三篇，是孤品，极为珍贵。来源于甲戌本之前不成熟本的是：25，26，27，28，35，36，37，39，75这9回。因未成熟本大部丢失，所以这9回有很高价值。

5. 戚序本：
* 戚蓼生，浙江湖州德清县人，清代乾隆年间的进士，于乾隆三十五年(1770年)至乾隆四十五年(1780年)在京做官。这段时间他整理出了抄录有脂批的《石头记》，并为之作序，只有前40回，有脂评，是老三篇的混合版。

6. 蒙府本：
* 书名《石头记》，1960年发现于清代一蒙古王府。现存国家图书馆，专用抄纸原抄部分74回，他人白纸抄补成120回。主要源自戚序本。
(def)
7. 列藏本：又称脂亚本，无书前题页。因藏于原苏联亚洲人民研究所列宁格勒分所，故名列藏本，是1962年李福清发现的。现存俄罗斯彼得堡东方研究所。存78回，缺5，6回。没有总书名。除少数几回名红楼梦外，各回皆名石头记。大部分抄自老三篇，有少量来源于未成熟本。

8. 舒序本：
* 书名《舒元炜序本红楼梦》，己酉本，存1-40回，抄于1798年，乾隆54年，是唯一有明确抄录时间的本子，所以有特殊价值，由吴晓铃收藏，除此以外，其他抄本都没有明确的抄录时间。

9. 卞藏本：
* 2006年发现，是早于甲戌本的不成熟本，很珍贵。有1-10回，和33-80回目录。

10. 甲辰本：
* 梦觉主人序本，1784年抄本，少量抄自老三篇，大部分源自程本，价值不大。1953年在山西发现，现存国家图书馆。

郑藏本：
* 
* 
* 

* 郑振铎藏本，只有

[-- Attachment #3: Type: text/plain, Size: 5 bytes --]


-- 

Re: "Final" version of tty child frames

[Gerd Möllmann]

Feng Shu <tumashu@163.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Feng Shu <tumashu@163.com> writes:
>>
>>> border do not work well when current buffer is CJK, maybe because CJK
>>> Char width is not 1
>>
>> Could you please send me a bit of CJK text so that I have something I
>> can test this with?

Thanks!

Re: "Final" version of tty child frames

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Feng Shu <tumashu@163.com> writes:
>
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>
>>> Feng Shu <tumashu@163.com> writes:
>>>
>>>> border do not work well when current buffer is CJK, maybe because CJK
>>>> Char width is not 1
>>>
>>> Could you please send me a bit of CJK text so that I have something I
>>> can test this with?
>
> Thanks!

Hm, okay, but not sure how to fix that.

The problem comes indeed from the root frame displaying characters that
are more then 1 column wide, say N. When we output such a character to
the terminal, the cursor moves by N columns. Emacs represents that in
the glyph matrix by producing N glyphs for the character, all bu tthe
first have a padding flag set. So we have

  ... (C, 0) (C, 1) ...

in the glpyh matrix, where C is the character, and the number is the
padding flag.

Now assume, for example, that the child frame is posititoned so that its
left border where we want to display a '|' is in the column for the (C,
1).

That can't work, because writing C to the terminal moves 2 columns and 
will skip over the column where we want the '|' to appear.

I think the same should happen, in principle, without borders. Haven't
checked that though. Maybe we have to "clear the field" to the left and
right of child frames in the root frame matrix so that wide characters
don't interfere.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Feng Shu <tumashu@163.com> writes:
>>
>>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>>
>>>> Feng Shu <tumashu@163.com> writes:
>>>>
>>>>> border do not work well when current buffer is CJK, maybe because CJK
>>>>> Char width is not 1
>>>>
>>>> Could you please send me a bit of CJK text so that I have something I
>>>> can test this with?
>>
>> Thanks!
>
> Hm, okay, but not sure how to fix that.
>
> The problem comes indeed from the root frame displaying characters that
> are more then 1 column wide, say N. When we output such a character to
> the terminal, the cursor moves by N columns. Emacs represents that in
> the glyph matrix by producing N glyphs for the character, all bu tthe
> first have a padding flag set. So we have
>
>   ... (C, 0) (C, 1) ...
>
> in the glpyh matrix, where C is the character, and the number is the
> padding flag.
>
> Now assume, for example, that the child frame is posititoned so that its
> left border where we want to display a '|' is in the column for the (C,
> 1).
>
> That can't work, because writing C to the terminal moves 2 columns and 
> will skip over the column where we want the '|' to appear.
>
> I think the same should happen, in principle, without borders. Haven't
> checked that though. Maybe we have to "clear the field" to the left and
> right of child frames in the root frame matrix so that wide characters
> don't interfere.

Another question is if there is something lurking with R2L? I have no
idea what terminals do in that case, but I'd guess Eli knows :-).

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Date: Wed, 23 Oct 2024 07:00:03 +0200
> 
> Another question is if there is something lurking with R2L? I have no
> idea what terminals do in that case, but I'd guess Eli knows :-).

Terminals should do nothing, and if your terminal emulator has a "bidi
reordering" feature, you should disable it, because Emacs does
everything by itself, and needs the terminal to be "dumb" in this
aspect.

Basically, we produce a glyph row where all the glyphs are flushed to
the right window edge, and there's empty space on the left of each
screen line to make it a full-width screen line.  The low-level write
code that writes to the terminal then works normally, writing glyphs
left to right.

The current branch hits an assertion violation when showing RTL text
in a child window.  (Showing RTL text in the parent window works
fine.)  Here's the recipe:

 emacs -Q -nw
 Evaluate in *scratch:

    (defun my-make-child ()
      (interactive)
      (make-frame `((parent-frame . ,(selected-frame))
		    (background-color . "gray10")
		    (foreground-color . "white")
		    (internal-border-width . 1)
		    (top . 15)
		    (left . 40)
		    (width . 80)
		    (height . 25))))

 M-x my-make-child RET
 C-x 5 o
 C-x C-f etc/tutorials/TUTORIAL.he RET

The backtrace from the assertion is below, including the data which is
involved.  As you see, X is 119 whereas the matrix_w is 108, which
violates the assertion.

Let me know if you want me to try something or show more data or maybe
tell you more about how RTL display works in TTY case.

  Thread 1 "emacs" received signal SIGABRT, Aborted.
  0x00007ff5201959fc in pthread_kill () from /lib/x86_64-linux-gnu/libc.so.6
  (gdb) bt
  #0  0x00007ff5201959fc in pthread_kill () at /lib/x86_64-linux-gnu/libc.so.6
  #1  0x00007ff520141476 in raise () at /lib/x86_64-linux-gnu/libc.so.6
  #2  0x0000559f4892c840 in terminate_due_to_signal
      (sig=sig@entry=6, backtrace_limit=backtrace_limit@entry=2147483647)
      at emacs.c:469
  #3  0x0000559f48933221 in die
      (msg=msg@entry=0x559f48c2ceb8 "x < root->current_matrix->matrix_w", file=file@entry=0x559f48c2c004 "dispnew.c", line=line@entry=3774) at alloc.c:8059
  #4  0x0000559f4891c4cb in is_cursor_obscured () at dispnew.c:3774
  #5  terminal_cursor_magic (topmost_child=0x559f4b07b9b0, root=0x559f4afcd050)
      at dispnew.c:3790
  #6  combine_updates_for_frame
      (f=<optimized out>, force_p=<optimized out>, inhibit_scrolling=<optimized out>) at dispnew.c:3840
  #7  0x0000559f48954656 in combine_updates
      (roots=roots@entry=0x7ff51c751723, force_p=force_p@entry=false, inhibit_scrolling=inhibit_scrolling@entry=false) at dispnew.c:3867
  #8  0x0000559f489af858 in redisplay_internal () at xdisp.c:17612
  #9  0x0000559f48abe24a in read_char
      (commandflag=1, map=0x7ff51c752503, prev_event=0x0, used_mouse_menu=0x7fff251757eb, end_time=0x0) at keyboard.c:2673
  #10 0x0000559f48abf794 in read_key_sequence
      (keybuf=0x7fff25175940, prompt=0x0, dont_downcase_last=<optimized out>, can_return_switch_frame=true, fix_current_buffer=true, prevent_redisplay=<optimized out>, disable_text_conversion_p=false) at keyboard.c:10747
  #11 0x0000559f48ac14f1 in command_loop_1 () at keyboard.c:1424
  #12 0x0000559f48b4fee7 in internal_condition_case
      (bfun=bfun@entry=0x559f48ac1280 <command_loop_1>, handlers=handlers@entry=0x90, hfun=hfun@entry=0x559f48ab4d50 <cmd_error>) at eval.c:1598
  #13 0x0000559f48aaab8e in command_loop_2 (handlers=handlers@entry=0x90)
      at keyboard.c:1163
  #14 0x0000559f48b4fd69 in internal_catch
      (tag=tag@entry=0x12360, func=func@entry=0x559f48aaab60 <command_loop_2>, arg=arg@entry=0x90) at eval.c:1277
  #15 0x0000559f48aaab29 in command_loop () at keyboard.c:1141
  #16 0x0000559f48ab4805 in recursive_edit_1 () at keyboard.c:749
  #17 0x0000559f48ab4bb8 in Frecursive_edit () at keyboard.c:832
  #18 0x0000559f489479e6 in main (argc=<optimized out>, argv=<optimized out>)
      at emacs.c:2624
  (gdb) up
  #1  0x00007fd8d9b21476 in raise () from /lib/x86_64-linux-gnu/libc.so.6
  (gdb)
  #2  0x000055ae22b9a840 in terminate_due_to_signal (sig=sig@entry=6,
      backtrace_limit=backtrace_limit@entry=2147483647) at emacs.c:469
  469       emacs_raise (sig);
  (gdb)
  #3  0x000055ae22ba1221 in die (
      msg=msg@entry=0x55ae22e9aeb8 "x < root->current_matrix->matrix_w",
      file=file@entry=0x55ae22e9a004 "dispnew.c", line=line@entry=3774)
      at alloc.c:8059
  8059      terminate_due_to_signal (SIGABRT, INT_MAX);
  (gdb)
  #4  0x000055ae22b8a4cb in is_cursor_obscured () at dispnew.c:3774
  3774      eassert (x < root->current_matrix->matrix_w);
  (gdb) p x
  $1 = 119
  (gdb) p root->current_matrix->matrix_w
  value has been optimized out
  (gdb) p root->current_matrix
  value has been optimized out
  (gdb) p root
  $2 = <optimized out>
  (gdb) up
  #5  terminal_cursor_magic (topmost_child=0x55ae245e89e8, root=0x55ae245c3050)
      at dispnew.c:3790
  3790      if (is_cursor_obscured ())
  (gdb) p root->current_matrix->matrix_w
  $3 = 108

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: Emacs Devel <emacs-devel@gnu.org>
>> Date: Wed, 23 Oct 2024 07:00:03 +0200
>> 
>> Another question is if there is something lurking with R2L? I have no
>> idea what terminals do in that case, but I'd guess Eli knows :-).
>
> Terminals should do nothing, and if your terminal emulator has a "bidi
> reordering" feature, you should disable it, because Emacs does
> everything by itself, and needs the terminal to be "dumb" in this
> aspect.
>
> Basically, we produce a glyph row where all the glyphs are flushed to
> the right window edge, and there's empty space on the left of each
> screen line to make it a full-width screen line.  The low-level write
> code that writes to the terminal then works normally, writing glyphs
> left to right.

Thanks, that's good to know for sure.

>
> The current branch hits an assertion violation when showing RTL text
> in a child window.  (Showing RTL text in the parent window works
> fine.)  Here's the recipe:
>
>  emacs -Q -nw
>  Evaluate in *scratch:
>
>     (defun my-make-child ()
>       (interactive)
>       (make-frame `((parent-frame . ,(selected-frame))
> 		    (background-color . "gray10")
> 		    (foreground-color . "white")
> 		    (internal-border-width . 1)
> 		    (top . 15)
> 		    (left . 40)
> 		    (width . 80)
> 		    (height . 25))))
>
>  M-x my-make-child RET
>  C-x 5 o
>  C-x C-f etc/tutorials/TUTORIAL.he RET

I'll have a look, a bit later.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> The current branch hits an assertion violation when showing RTL text
> in a child window.  (Showing RTL text in the parent window works
> fine.)  Here's the recipe:
>
>  emacs -Q -nw
>  Evaluate in *scratch:
>
>     (defun my-make-child ()
>       (interactive)
>       (make-frame `((parent-frame . ,(selected-frame))
> 		    (background-color . "gray10")
> 		    (foreground-color . "white")
> 		    (internal-border-width . 1)
> 		    (top . 15)
> 		    (left . 40)
> 		    (width . 80)
> 		    (height . 25))))
>
>  M-x my-make-child RET
>  C-x 5 o
>  C-x C-f etc/tutorials/TUTORIAL.he RET

Could you please check with what I pushed now? There were several
problem, starting with an error I made while porting this, plus thinkos.
Strangely, I didn't run into the assertion with the recipe, so it's
probably best if you check yourself if it works now.

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: tumashu@163.com,  emacs-devel@gnu.org
> Date: Wed, 23 Oct 2024 13:04:17 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > The current branch hits an assertion violation when showing RTL text
> > in a child window.  (Showing RTL text in the parent window works
> > fine.)  Here's the recipe:
> >
> >  emacs -Q -nw
> >  Evaluate in *scratch:
> >
> >     (defun my-make-child ()
> >       (interactive)
> >       (make-frame `((parent-frame . ,(selected-frame))
> > 		    (background-color . "gray10")
> > 		    (foreground-color . "white")
> > 		    (internal-border-width . 1)
> > 		    (top . 15)
> > 		    (left . 40)
> > 		    (width . 80)
> > 		    (height . 25))))
> >
> >  M-x my-make-child RET
> >  C-x 5 o
> >  C-x C-f etc/tutorials/TUTORIAL.he RET
> 
> Could you please check with what I pushed now? There were several
> problem, starting with an error I made while porting this, plus thinkos.
> Strangely, I didn't run into the assertion with the recipe, so it's
> probably best if you check yourself if it works now.

The assertion violation is gone, and the display seems correct, but I
now get compilation warning:

    CC       dispnew.o
  In file included from dispnew.c:27:
  dispnew.c: In function ‘combine_updates_for_frame’:
  lisp.h:1096:68: warning: null pointer dereference [-Wnull-dereference]
   1096 |           && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
	|                                                                    ^

I have no idea what is GCC talking about.

Thanks.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

> The assertion violation is gone, and the display seems correct, but I
> now get compilation warning:
>
>     CC       dispnew.o
>   In file included from dispnew.c:27:
>   dispnew.c: In function ‘combine_updates_for_frame’:
>   lisp.h:1096:68: warning: null pointer dereference [-Wnull-dereference]
>    1096 |           && ((XUNTAG (a, Lisp_Vectorlike, union vectorlike_header)->size
> 	|                                                                    ^
>
> I have no idea what is GCC talking about.
>
> Thanks.

Thanks for checking!

I don't have an idea what GCC means either. Can one perhaps turn off
inlining for the compilation? Maybe GCC then prints something less
cryptic.

Re: "Final" version of tty child frames

[Feng Shu]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Feng Shu <tumashu@163.com> writes:
>>
>>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>>
>>>> Feng Shu <tumashu@163.com> writes:
>>>>
>>>>> border do not work well when current buffer is CJK, maybe because CJK
>>>>> Char width is not 1
>>>>
>>>> Could you please send me a bit of CJK text so that I have something I
>>>> can test this with?
>>
>> Thanks!
>
> Hm, okay, but not sure how to fix that.
>

This is a bad news, if we can not fix this problem, enable this feature
all the world will be not a good idea.



> The problem comes indeed from the root frame displaying characters that
> are more then 1 column wide, say N. When we output such a character to
> the terminal, the cursor moves by N columns. Emacs represents that in
> the glyph matrix by producing N glyphs for the character, all bu tthe
> first have a padding flag set. So we have
>
>   ... (C, 0) (C, 1) ...
>
> in the glpyh matrix, where C is the character, and the number is the
> padding flag.
>
> Now assume, for example, that the child frame is posititoned so that its
> left border where we want to display a '|' is in the column for the (C,
> 1).
>
> That can't work, because writing C to the terminal moves 2 columns and 
> will skip over the column where we want the '|' to appear.
>
> I think the same should happen, in principle, without borders. Haven't
> checked that though. Maybe we have to "clear the field" to the left and
> right of child frames in the root frame matrix so that wide characters
> don't interfere.

-- 

Re: "Final" version of tty child frames

[Gerd Möllmann]

Feng Shu <tumashu@163.com> writes:

>> Hm, okay, but not sure how to fix that.
>
> This is a bad news, if we can not fix this problem, enable this feature
> all the world will be not a good idea.

Don't be pessinistic :-). I'm just figuring out what's the best way to
fix it. It's not that I don't have ideas.

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Date: Wed, 23 Oct 2024 06:40:11 +0200
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> > Feng Shu <tumashu@163.com> writes:
> >
> >> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> >>
> >>> Feng Shu <tumashu@163.com> writes:
> >>>
> >>>> border do not work well when current buffer is CJK, maybe because CJK
> >>>> Char width is not 1
> >>>
> >>> Could you please send me a bit of CJK text so that I have something I
> >>> can test this with?
> >
> > Thanks!
> 
> Hm, okay, but not sure how to fix that.
> 
> The problem comes indeed from the root frame displaying characters that
> are more then 1 column wide, say N. When we output such a character to
> the terminal, the cursor moves by N columns. Emacs represents that in
> the glyph matrix by producing N glyphs for the character, all bu tthe
> first have a padding flag set. So we have
> 
>   ... (C, 0) (C, 1) ...
> 
> in the glpyh matrix, where C is the character, and the number is the
> padding flag.
> 
> Now assume, for example, that the child frame is posititoned so that its
> left border where we want to display a '|' is in the column for the (C,
> 1).
> 
> That can't work, because writing C to the terminal moves 2 columns and 
> will skip over the column where we want the '|' to appear.

I think the solution should be the same as what we do when a line
needs to wrap (i.e. be continued) and we don't have enough columns to
put all the N glyphs before the continuation glyph: we don't show the
entire N-glyph group.  IOW, the border glyph should be shown instead
of multi-column character, padded with enough spaces before it to make
up for the M < N glyphs of C that could be shown before the border.
In your example above, C will not be shown, and instead we will show
one space glyph followed by the '|' glyph.

Re: "Final" version of tty child frames

[Gerd Möllmann]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: Emacs Devel <emacs-devel@gnu.org>
>> Date: Wed, 23 Oct 2024 06:40:11 +0200
>> 
>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>> 
>> > Feng Shu <tumashu@163.com> writes:
>> >
>> >> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>> >>
>> >>> Feng Shu <tumashu@163.com> writes:
>> >>>
>> >>>> border do not work well when current buffer is CJK, maybe because CJK
>> >>>> Char width is not 1
>> >>>
>> >>> Could you please send me a bit of CJK text so that I have something I
>> >>> can test this with?
>> >
>> > Thanks!
>> 
>> Hm, okay, but not sure how to fix that.
>> 
>> The problem comes indeed from the root frame displaying characters that
>> are more then 1 column wide, say N. When we output such a character to
>> the terminal, the cursor moves by N columns. Emacs represents that in
>> the glyph matrix by producing N glyphs for the character, all bu tthe
>> first have a padding flag set. So we have
>> 
>>   ... (C, 0) (C, 1) ...
>> 
>> in the glpyh matrix, where C is the character, and the number is the
>> padding flag.
>> 
>> Now assume, for example, that the child frame is posititoned so that its
>> left border where we want to display a '|' is in the column for the (C,
>> 1).
>> 
>> That can't work, because writing C to the terminal moves 2 columns and 
>> will skip over the column where we want the '|' to appear.
>
> I think the solution should be the same as what we do when a line
> needs to wrap (i.e. be continued) and we don't have enough columns to
> put all the N glyphs before the continuation glyph: we don't show the
> entire N-glyph group.  IOW, the border glyph should be shown instead
> of multi-column character, padded with enough spaces before it to make
> up for the M < N glyphs of C that could be shown before the border.
> In your example above, C will not be shown, and instead we will show
> one space glyph followed by the '|' glyph.

Yes, I came up with this (WIP):

/* If the glyph in ROW at position X is part of a wide character, change
   every glyph belonging to the wide character to a space glyph.  */

static void
neutralize_wide_char (struct frame *root, struct glyph_row *row, int x)
{
  eassert (x >= 0 && x < root->desired_matrix->matrix_w);
  struct glyph *glyph = row->glyphs[0] + x;
  if (glyph->type == CHAR_GLYPH && CHARACTER_WIDTH (glyph->u.ch) > 1)
    {
      struct glyph *row_start = row->glyphs[0];
      struct glyph *row_limit = row_start + row->used[0];

      /* Glyph is somewhere in a sequence of glyphs for a wide
	 character, find the start.  */
      while (glyph > row_start && glyph->padding_p)
	--glyph;

      /* Make everything in the sequence a space glyph.  */
      eassert (!glyph->padding_p);
      make_glyph_space (glyph);
      for (++glyph; glyph < row_limit && glyph->padding_p; ++glyph)
	make_glyph_space (glyph);
    }
}

I have some problems reproducing the whole thing on my system with the
CJK file Feng Shu gave me. Hm.

Re: "Final" version of tty child frames

[Feng Shu]

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

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>>> Cc: Emacs Devel <emacs-devel@gnu.org>
>>> Date: Wed, 23 Oct 2024 06:40:11 +0200
>>> 
>>> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>> 
>>> > Feng Shu <tumashu@163.com> writes:
>>> >
>>> >> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>>> >>
>>> >>> Feng Shu <tumashu@163.com> writes:
>>> >>>
>>> >>>> border do not work well when current buffer is CJK, maybe because CJK
>>> >>>> Char width is not 1
>>> >>>
>>> >>> Could you please send me a bit of CJK text so that I have something I
>>> >>> can test this with?
>>> >
>>> > Thanks!
>>> 
>>> Hm, okay, but not sure how to fix that.
>>> 
>>> The problem comes indeed from the root frame displaying characters that
>>> are more then 1 column wide, say N. When we output such a character to
>>> the terminal, the cursor moves by N columns. Emacs represents that in
>>> the glyph matrix by producing N glyphs for the character, all bu tthe
>>> first have a padding flag set. So we have
>>> 
>>>   ... (C, 0) (C, 1) ...
>>> 
>>> in the glpyh matrix, where C is the character, and the number is the
>>> padding flag.
>>> 
>>> Now assume, for example, that the child frame is posititoned so that its
>>> left border where we want to display a '|' is in the column for the (C,
>>> 1).
>>> 
>>> That can't work, because writing C to the terminal moves 2 columns and 
>>> will skip over the column where we want the '|' to appear.
>>
>> I think the solution should be the same as what we do when a line
>> needs to wrap (i.e. be continued) and we don't have enough columns to
>> put all the N glyphs before the continuation glyph: we don't show the
>> entire N-glyph group.  IOW, the border glyph should be shown instead
>> of multi-column character, padded with enough spaces before it to make
>> up for the M < N glyphs of C that could be shown before the border.
>> In your example above, C will not be shown, and instead we will show
>> one space glyph followed by the '|' glyph.
>
> Yes, I came up with this (WIP):
>
> /* If the glyph in ROW at position X is part of a wide character, change
>    every glyph belonging to the wide character to a space glyph.  */
>
> static void
> neutralize_wide_char (struct frame *root, struct glyph_row *row, int x)
> {
>   eassert (x >= 0 && x < root->desired_matrix->matrix_w);
>   struct glyph *glyph = row->glyphs[0] + x;
>   if (glyph->type == CHAR_GLYPH && CHARACTER_WIDTH (glyph->u.ch) > 1)
>     {
>       struct glyph *row_start = row->glyphs[0];
>       struct glyph *row_limit = row_start + row->used[0];
>
>       /* Glyph is somewhere in a sequence of glyphs for a wide
> 	 character, find the start.  */
>       while (glyph > row_start && glyph->padding_p)
> 	--glyph;
>
>       /* Make everything in the sequence a space glyph.  */
>       eassert (!glyph->padding_p);
>       make_glyph_space (glyph);
>       for (++glyph; glyph < row_limit && glyph->padding_p; ++glyph)
> 	make_glyph_space (glyph);
>     }
> }
>
> I have some problems reproducing the whole thing on my system with the
> CJK file Feng Shu gave me. Hm.

The below adjust will show more issue:-)

1. Reduce terminal's width
2. let a long line auto wrap to many lines.



[-- Attachment #2: 2024-10-23_15-51.png --]
[-- Type: image/png, Size: 129309 bytes --]

[-- Attachment #3: Type: text/plain, Size: 7 bytes --]




-- 

Re: "Final" version of tty child frames

[Gerd Möllmann]

Feng Shu <tumashu@163.com> writes:

> The below adjust will show more issue:-)
>
> 1. Reduce terminal's width
> 2. let a long line auto wrap to many lines.

Could you please see what happens with what I just pushed to savannah?
The other one, with the borders, and this one.

Re: "Final" version of tty child frames

[Feng Shu]

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

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Feng Shu <tumashu@163.com> writes:
>
>> The below adjust will show more issue:-)
>>
>> 1. Reduce terminal's width
>> 2. let a long line auto wrap to many lines.
>
> Could you please see what happens with what I just pushed to savannah?
> The other one, with the borders, and this one.

Works, thanks!


[-- Attachment #2: 2024-10-23_17-06.png --]
[-- Type: image/png, Size: 187772 bytes --]

[-- Attachment #3: Type: text/plain, Size: 7 bytes --]




-- 

Re: "Final" version of tty child frames

[Gerd Möllmann]

Feng Shu <tumashu@163.com> writes:

> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
>
>> Feng Shu <tumashu@163.com> writes:
>>
>>> The below adjust will show more issue:-)
>>>
>>> 1. Reduce terminal's width
>>> 2. let a long line auto wrap to many lines.
>>
>> Could you please see what happens with what I just pushed to savannah?
>> The other one, with the borders, and this one.
>
> Works, thanks!

Thanks!

Re: "Final" version of tty child frames

[Eli Zaretskii]

> From: Feng Shu <tumashu@163.com>
> Cc: Emacs Devel <emacs-devel@gnu.org>
> Date: Wed, 23 Oct 2024 11:05:51 +0800
> 
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
> Will the below interfaces changed?
> 
> 1. tty-child-frames
> 2. tty-non-selected-cursor
> 3. undecorated
> 
> If not, I have updated posframe, vertico-posframe and ivy-posframe to
> support three interfaces.

It's too early to make changes in 3rd-party packages based on what you
see on the branch.  The branch is too young for that, and it's quite
possible that Lisp APIs related to this feature will change, when
enough people take a look at what's there.

I suggest to wait for this branch to land on master before you make
public changes in your packages.  Until then, please provide your
inputs and feedback for using the branch, so that it takes your use
cases into consideration.

Re: "Final" version of tty child frames

[Gerd Möllmann]

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

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Other things not contained:
>
...
> - Tooltips. I think tooltips could relatively easily be implemented
>   with child frames by copying or extracting the non-native tooltip
>   code from x-show-tip or some such. From my POV, that would be best
>   done by refactoring the tooltip code across window systems which I
>   can't do.

Just a gimmick: sort of tooltips for displaying help on ttys,


[-- Attachment #2: tty-tip.el --]
[-- Type: application/emacs-lisp, Size: 5171 bytes --]