unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Question about dubious code for terminal frames
@ 2024-09-02  9:04 Gerd Möllmann
  2024-09-02  9:10 ` Andreas Schwab
  2024-09-02 11:44 ` Eli Zaretskii
  0 siblings, 2 replies; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02  9:04 UTC (permalink / raw)
  To: Emacs Devel; +Cc: martin rudalics

In 3 places, among them adjust_frame_size, which is why I CC'd
Martin, Emacs sets the values of FrameRows/FrameCols.

AFAIU, these two values give the physical size of a terminal (screen,
window), as returned from the co and li escape sequences on termcap
frames. And I can't find code that tries to change the physical size of
the terminal, if that would even make sense in the first place :-). So
this doesn't make any sense to me.

Alas, the code in question has been so heavily modified over the decades
that I can't find out where this code originates from. Maybe it's even
something that I missed to fix when introducing the new redisplay,
because in the old redisplay it might have mad>e sense for GUI frames.
Maybe, don't remember.

In any case, does someone have an idea what that code is for? If not,
I suggest to remove it (and maybe making them rvalues or const).



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02  9:04 Question about dubious code for terminal frames Gerd Möllmann
@ 2024-09-02  9:10 ` Andreas Schwab
  2024-09-02  9:48   ` Gerd Möllmann
  2024-09-02 11:44 ` Eli Zaretskii
  1 sibling, 1 reply; 23+ messages in thread
From: Andreas Schwab @ 2024-09-02  9:10 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: Emacs Devel, martin rudalics

On Sep 02 2024, Gerd Möllmann wrote:

> AFAIU, these two values give the physical size of a terminal (screen,
> window), as returned from the co and li escape sequences on termcap
> frames.

Only if get_tty_size cannot determine the real size.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02  9:10 ` Andreas Schwab
@ 2024-09-02  9:48   ` Gerd Möllmann
  2024-09-02 10:23     ` Andreas Schwab
  0 siblings, 1 reply; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02  9:48 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Emacs Devel, martin rudalics

Andreas Schwab <schwab@suse.de> writes:

> On Sep 02 2024, Gerd Möllmann wrote:
>
>> AFAIU, these two values give the physical size of a terminal (screen,
>> window), as returned from the co and li escape sequences on termcap
>> frames.
>
> Only if get_tty_size cannot determine the real size.

Hm, yes, thanks. I guess setting cols and rows should then be
conditionalized on being unknown?



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02  9:48   ` Gerd Möllmann
@ 2024-09-02 10:23     ` Andreas Schwab
  2024-09-02 10:46       ` Gerd Möllmann
  0 siblings, 1 reply; 23+ messages in thread
From: Andreas Schwab @ 2024-09-02 10:23 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: Emacs Devel, martin rudalics

On Sep 02 2024, Gerd Möllmann wrote:

> Andreas Schwab <schwab@suse.de> writes:
>
>> On Sep 02 2024, Gerd Möllmann wrote:
>>
>>> AFAIU, these two values give the physical size of a terminal (screen,
>>> window), as returned from the co and li escape sequences on termcap
>>> frames.
>>
>> Only if get_tty_size cannot determine the real size.
>
> Hm, yes, thanks. I guess setting cols and rows should then be
> conditionalized on being unknown?

  if (FrameCols (tty) <= 0)
    FrameCols (tty) = tgetnum ("co");
  if (FrameRows (tty) <= 0)
    FrameRows (tty) = tgetnum ("li");

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 10:23     ` Andreas Schwab
@ 2024-09-02 10:46       ` Gerd Möllmann
  0 siblings, 0 replies; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02 10:46 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Emacs Devel, martin rudalics

Andreas Schwab <schwab@suse.de> writes:

> On Sep 02 2024, Gerd Möllmann wrote:
>
>> Andreas Schwab <schwab@suse.de> writes:
>>
>>> On Sep 02 2024, Gerd Möllmann wrote:
>>>
>>>> AFAIU, these two values give the physical size of a terminal (screen,
>>>> window), as returned from the co and li escape sequences on termcap
>>>> frames.
>>>
>>> Only if get_tty_size cannot determine the real size.
>>
>> Hm, yes, thanks. I guess setting cols and rows should then be
>> conditionalized on being unknown?
>
>   if (FrameCols (tty) <= 0)
>     FrameCols (tty) = tgetnum ("co");
>   if (FrameRows (tty) <= 0)
>     FrameRows (tty) = tgetnum ("li");

I eean in adjust_frame_size

  if (new_inner_width != old_inner_width)
    {
      resize_frame_windows (f, new_inner_width, true);

      /* MSDOS frames cannot PRETEND, as they change frame size by
	 manipulating video hardware.  */
      if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
	FrameCols (FRAME_TTY (f)) = new_text_cols;



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02  9:04 Question about dubious code for terminal frames Gerd Möllmann
  2024-09-02  9:10 ` Andreas Schwab
@ 2024-09-02 11:44 ` Eli Zaretskii
  2024-09-02 12:59   ` Gerd Möllmann
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2024-09-02 11:44 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: emacs-devel, rudalics

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: martin rudalics <rudalics@gmx.at>
> Date: Mon, 02 Sep 2024 11:04:02 +0200
> 
> In 3 places, among them adjust_frame_size, which is why I CC'd
> Martin, Emacs sets the values of FrameRows/FrameCols.
> 
> AFAIU, these two values give the physical size of a terminal (screen,
> window), as returned from the co and li escape sequences on termcap
> frames. And I can't find code that tries to change the physical size of
> the terminal, if that would even make sense in the first place :-). So
> this doesn't make any sense to me.

Did you consider SIGWINCH?  Its handler also calls adjust_frame_size.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 11:44 ` Eli Zaretskii
@ 2024-09-02 12:59   ` Gerd Möllmann
  2024-09-02 13:17     ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02 12:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, rudalics

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: martin rudalics <rudalics@gmx.at>
>> Date: Mon, 02 Sep 2024 11:04:02 +0200
>> 
>> In 3 places, among them adjust_frame_size, which is why I CC'd
>> Martin, Emacs sets the values of FrameRows/FrameCols.
>> 
>> AFAIU, these two values give the physical size of a terminal (screen,
>> window), as returned from the co and li escape sequences on termcap
>> frames. And I can't find code that tries to change the physical size of
>> the terminal, if that would even make sense in the first place :-). So
>> this doesn't make any sense to me.
>
> Did you consider SIGWINCH?  Its handler also calls adjust_frame_size.

Thanks, that would be this one, right?

static void
handle_window_change_signal (int sig)
{
  int width, height;
  struct tty_display_info *tty;

  /* The frame size change obviously applies to a single
     termcap-controlled terminal, but we can't decide which.
     Therefore, we resize the frames corresponding to each tty.
  */
  for (tty = tty_list; tty; tty = tty->next)
    {
      if (! tty->term_initted)
	continue;

      /* Suspended tty frames have tty->input == NULL avoid trying to
	 use it.  */
      if (!tty->input)
	continue;

      get_tty_size (fileno (tty->input), &width, &height);

      if (width > 5 && height > 2)
	{
	  Lisp_Object tail, frame;

	  FOR_EACH_FRAME (tail, frame)
	    {
	      struct frame *f = XFRAME (frame);

	      if (FRAME_TERMCAP_P (f) && FRAME_TTY (f) == tty)
		/* Record the new sizes, but don't reallocate the data
		   structures now.  Let that be done later outside of the
		   signal handler.  */
		change_frame_size (f, width, height, false, true, false);
	    }
	}
    }
}

It also could know and set the terminal size, if get_tty_size can.

How complicated. I guess I rather work around this for now :-).

And I wonder if one could brew a make-frame with some suitable frame
parameters that lead to setting the terminal size to something that
leads to a later emacs_abort in cmcheckmagic during redisplay of another
frame on the same terminal :-).




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 12:59   ` Gerd Möllmann
@ 2024-09-02 13:17     ` Eli Zaretskii
  2024-09-02 14:11       ` Gerd Möllmann
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2024-09-02 13:17 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: emacs-devel, rudalics

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
> Date: Mon, 02 Sep 2024 14:59:54 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Did you consider SIGWINCH?  Its handler also calls adjust_frame_size.
> 
> Thanks, that would be this one, right?

Right.

> It also could know and set the terminal size, if get_tty_size can.
> 
> How complicated. I guess I rather work around this for now :-).

What exactly is the problem?  Doesn't change_frame_size support child
frames?

> And I wonder if one could brew a make-frame with some suitable frame
> parameters that lead to setting the terminal size to something that
> leads to a later emacs_abort in cmcheckmagic during redisplay of another
> frame on the same terminal :-).

Someone already did, see bug#71289.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 13:17     ` Eli Zaretskii
@ 2024-09-02 14:11       ` Gerd Möllmann
  2024-09-02 14:35         ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02 14:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, rudalics

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
>> Date: Mon, 02 Sep 2024 14:59:54 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Did you consider SIGWINCH?  Its handler also calls adjust_frame_size.
>> 
>> Thanks, that would be this one, right?
>
> Right.
>
>> It also could know and set the terminal size, if get_tty_size can.
>> 
>> How complicated. I guess I rather work around this for now :-).
>
> What exactly is the problem?  Doesn't change_frame_size support child
> frames?

Not on terminals, no, for exactly the reason that adjust_frame_size for
terminal frames sets the terminal's size. So, when I opened a new child
frame of size 20x30 suddenty the terminal had size 20x30. And the
innocent root frame redisplay got an emacs_abort because it wrote
outside of that range because the root frame was of course much larger.

>> And I wonder if one could brew a make-frame with some suitable frame
>> parameters that lead to setting the terminal size to something that
>> leads to a later emacs_abort in cmcheckmagic during redisplay of another
>> frame on the same terminal :-).
>
> Someone already did, see bug#71289.

Hahaha :-).

I personally think it's wrong to set FrameCols/Rows in this way in
adjust_frame_size. From my POV, it's should be a physical property of
the terminal that Emacs does not change. If we can't determine the size
of the terminal (and I wonder how often that is the case), we should
devise a way to let the user specify the size, but without the current
magic. 



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 14:11       ` Gerd Möllmann
@ 2024-09-02 14:35         ` Eli Zaretskii
  2024-09-02 14:54           ` Gerd Möllmann
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2024-09-02 14:35 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: emacs-devel, rudalics

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
> Date: Mon, 02 Sep 2024 16:11:34 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > What exactly is the problem?  Doesn't change_frame_size support child
> > frames?
> 
> Not on terminals, no, for exactly the reason that adjust_frame_size for
> terminal frames sets the terminal's size. So, when I opened a new child
> frame of size 20x30 suddenty the terminal had size 20x30. And the
> innocent root frame redisplay got an emacs_abort because it wrote
> outside of that range because the root frame was of course much larger.

OK, but the behavior on TTY terminals in this regard should be exactly
the same as on GUI terminals, when you drag the frame's border with
the mouse.  So I very much hope there's code in Emacs that you can
simply copy or maybe even just call.

> I personally think it's wrong to set FrameCols/Rows in this way in
> adjust_frame_size. From my POV, it's should be a physical property of
> the terminal that Emacs does not change. If we can't determine the size
> of the terminal (and I wonder how often that is the case), we should
> devise a way to let the user specify the size, but without the current
> magic. 

But with the current proliferation of terminal emulators whose window
can be resized, what other choice do we have when the only source of
information about the resize is SIGWINCH?  How else can we update the
Emacs notion of the frame's size?



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 14:35         ` Eli Zaretskii
@ 2024-09-02 14:54           ` Gerd Möllmann
  2024-09-02 15:31             ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02 14:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, rudalics

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
>> Date: Mon, 02 Sep 2024 16:11:34 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > What exactly is the problem?  Doesn't change_frame_size support child
>> > frames?
>> 
>> Not on terminals, no, for exactly the reason that adjust_frame_size for
>> terminal frames sets the terminal's size. So, when I opened a new child
>> frame of size 20x30 suddenty the terminal had size 20x30. And the
>> innocent root frame redisplay got an emacs_abort because it wrote
>> outside of that range because the root frame was of course much larger.
>
> OK, but the behavior on TTY terminals in this regard should be exactly
> the same as on GUI terminals, when you drag the frame's border with
> the mouse.  So I very much hope there's code in Emacs that you can
> simply copy or maybe even just call.

Well, an easy cop-out is to not set FrameCols/FrameRows for child
frames. Seems to work, but it's not a solution. Hence my question here.

>> I personally think it's wrong to set FrameCols/Rows in this way in
>> adjust_frame_size. From my POV, it's should be a physical property of
>> the terminal that Emacs does not change. If we can't determine the size
>> of the terminal (and I wonder how often that is the case), we should
>> devise a way to let the user specify the size, but without the current
>> magic. 
>
> But with the current proliferation of terminal emulators whose window
> can be resized, what other choice do we have when the only source of
> information about the resize is SIGWINCH?  How else can we update the
> Emacs notion of the frame's size?

See get_tty_size which the SIGWINCH handler already calls. We could use
that to get the physical dimension of the terminal. Only problem are
terminals not supporting one of the methods get_tty_size knows, for
example the co/li escape sequences. We'd need some way to let the user
specify the terminal size for that case.

Outside of get_tty_size I'd make FrameCols/Rows const. I guess that
frame size changes could remain as is. Only that setting FrameCols/Rows
is none of their business. It's an input for them, not an output.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 14:54           ` Gerd Möllmann
@ 2024-09-02 15:31             ` Eli Zaretskii
  2024-09-02 15:46               ` Gerd Möllmann
  2024-09-02 16:20               ` martin rudalics
  0 siblings, 2 replies; 23+ messages in thread
From: Eli Zaretskii @ 2024-09-02 15:31 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: emacs-devel, rudalics

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
> Date: Mon, 02 Sep 2024 16:54:31 +0200
> 
> > But with the current proliferation of terminal emulators whose window
> > can be resized, what other choice do we have when the only source of
> > information about the resize is SIGWINCH?  How else can we update the
> > Emacs notion of the frame's size?
> 
> See get_tty_size which the SIGWINCH handler already calls. We could use
> that to get the physical dimension of the terminal.

Get the physical dimensions and then what?  We must record the
dimensions in the frames and windows that share that terminal.  Which
is what change_frame_size does.

> Outside of get_tty_size I'd make FrameCols/Rows const. I guess that
> frame size changes could remain as is. Only that setting FrameCols/Rows
> is none of their business. It's an input for them, not an output.

Martin will correct me if I'm wrong, but I think the current code
already does what you want: it only changes FrameCols/Rows when the
terminal was really resized.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 15:31             ` Eli Zaretskii
@ 2024-09-02 15:46               ` Gerd Möllmann
  2024-09-02 15:55                 ` Eli Zaretskii
  2024-09-02 16:20               ` martin rudalics
  1 sibling, 1 reply; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02 15:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, rudalics

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
>> Date: Mon, 02 Sep 2024 16:54:31 +0200
>> 
>> > But with the current proliferation of terminal emulators whose window
>> > can be resized, what other choice do we have when the only source of
>> > information about the resize is SIGWINCH?  How else can we update the
>> > Emacs notion of the frame's size?
>> 
>> See get_tty_size which the SIGWINCH handler already calls. We could use
>> that to get the physical dimension of the terminal.
>
> Get the physical dimensions and then what?  We must record the
> dimensions in the frames and windows that share that terminal.  Which
> is what change_frame_size does.

Set the terminal size And then do what we currently do to adapt the
frame sizes. Only that the terminal size%1b[<35;19;43M now an input and
not the result of frame size changes.

>
>> Outside of get_tty_size I'd make FrameCols/Rows const. I guess that
>> frame size changes could remain as is. Only that setting FrameCols/Rows
>> is none of their business. It's an input for them, not an output.
>
> Martin will correct me if I'm wrong, but I think the current code
> already does what you want: it only changes FrameCols/Rows when the
> terminal was really resized.

It obviously does not. I mentioned what happens when making child frames
in adjust_frame_size in my case. There is no SIGWINCH involved. Which
makes me suspect that you are not taking adjust_frame_size into
account?







^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 15:46               ` Gerd Möllmann
@ 2024-09-02 15:55                 ` Eli Zaretskii
  2024-09-02 16:24                   ` Gerd Möllmann
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2024-09-02 15:55 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: emacs-devel, rudalics

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
> Date: Mon, 02 Sep 2024 17:46:43 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Martin will correct me if I'm wrong, but I think the current code
> > already does what you want: it only changes FrameCols/Rows when the
> > terminal was really resized.
> 
> It obviously does not. I mentioned what happens when making child frames
> in adjust_frame_size in my case. There is no SIGWINCH involved. Which
> makes me suspect that you are not taking adjust_frame_size into
> account?

I wasn't talking about child frames, I was talking about "normal"
frames.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 15:31             ` Eli Zaretskii
  2024-09-02 15:46               ` Gerd Möllmann
@ 2024-09-02 16:20               ` martin rudalics
  2024-09-02 16:31                 ` Gerd Möllmann
  1 sibling, 1 reply; 23+ messages in thread
From: martin rudalics @ 2024-09-02 16:20 UTC (permalink / raw)
  To: Eli Zaretskii, Gerd Möllmann; +Cc: emacs-devel

 > Get the physical dimensions and then what?  We must record the
 > dimensions in the frames and windows that share that terminal.  Which
 > is what change_frame_size does.
 >
 >> Outside of get_tty_size I'd make FrameCols/Rows const. I guess that
 >> frame size changes could remain as is. Only that setting FrameCols/Rows
 >> is none of their business. It's an input for them, not an output.
 >
 > Martin will correct me if I'm wrong, but I think the current code
 > already does what you want: it only changes FrameCols/Rows when the
 > terminal was really resized.

I started reading this thread just now and I have to go through the
preceding posts to even understand the problem.  The only thing I can
say at the moment is that I locally made a change to adjust_frame_size
where I moved the assignment of the new frame values headed by

   /* Assign new sizes.  */

up in the code.  The context is now as:


   dos_set_window_size (&dos_new_text_lines, &new_text_cols);
   new_text_lines = dos_new_text_lines - FRAME_TOP_MARGIN (f);
#endif

   /* Assign new sizes.  Do it here to make sure that frame based
      redisplay gets congruent sizes for the dimensions of the frame
      matrix and the combined window matrices.  */
   FRAME_COLS (f) = new_text_cols;
   FRAME_LINES (f) = new_text_lines;
   FRAME_TEXT_WIDTH (f) = new_text_width;
   FRAME_TEXT_HEIGHT (f) = new_text_height;
   FRAME_PIXEL_WIDTH (f) = new_native_width;
   FRAME_PIXEL_HEIGHT (f) = new_native_height;
   FRAME_TOTAL_COLS (f) = FRAME_PIXEL_WIDTH (f) / FRAME_COLUMN_WIDTH (f);
   FRAME_TOTAL_LINES (f) = FRAME_PIXEL_HEIGHT (f) / FRAME_LINE_HEIGHT (f);

   if (new_inner_width != old_inner_width)
     {
       resize_frame_windows (f, new_inner_width, true);


 From the modified comment I conjecture that there must have been some
problem with frame based redisplay but I can't recall what it was and
whether it's relevant for the problems in this thread.

martin



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 15:55                 ` Eli Zaretskii
@ 2024-09-02 16:24                   ` Gerd Möllmann
  2024-09-02 16:35                     ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02 16:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, rudalics

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
>> Date: Mon, 02 Sep 2024 17:46:43 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Martin will correct me if I'm wrong, but I think the current code
>> > already does what you want: it only changes FrameCols/Rows when the
>> > terminal was really resized.
>> 
>> It obviously does not. I mentioned what happens when making child frames
>> in adjust_frame_size in my case. There is no SIGWINCH involved. Which
>> makes me suspect that you are not taking adjust_frame_size into
>> account?
>
> I wasn't talking about child frames, I was talking about "normal"
> frames.

We're taking past each other.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 16:20               ` martin rudalics
@ 2024-09-02 16:31                 ` Gerd Möllmann
  2024-09-02 17:32                   ` martin rudalics
  0 siblings, 1 reply; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02 16:31 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, emacs-devel

martin rudalics <rudalics@gmx.at> writes:

>> Get the physical dimensions and then what?  We must record the
>> dimensions in the frames and windows that share that terminal.  Which
>> is what change_frame_size does.
>>
>>> Outside of get_tty_size I'd make FrameCols/Rows const. I guess that
>>> frame size changes could remain as is. Only that setting FrameCols/Rows
>>> is none of their business. It's an input for them, not an output.
>>
>> Martin will correct me if I'm wrong, but I think the current code
>> already does what you want: it only changes FrameCols/Rows when the
>> terminal was really resized.
>
> I started reading this thread just now and I have to go through the
> preceding posts to even understand the problem.  The only thing I can
> say at the moment is that I locally made a change to adjust_frame_size
> where I moved the assignment of the new frame values headed by
>
>   /* Assign new sizes.  */
>
> up in the code.  The context is now as:

I think I have the above here. It reads

  /* Assign new sizes.  */
  FRAME_COLS (f) = new_text_cols;
  FRAME_LINES (f) = new_text_lines;

Anyway. My problem is setting FrameCols/Rows in adjust_frame_size. It's
not guaranteed that the terminal's size has indeed changed when that is
done.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 16:24                   ` Gerd Möllmann
@ 2024-09-02 16:35                     ` Eli Zaretskii
  2024-09-02 16:38                       ` Gerd Möllmann
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2024-09-02 16:35 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: emacs-devel, rudalics

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
> Date: Mon, 02 Sep 2024 18:24:12 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> >> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
> >> Date: Mon, 02 Sep 2024 17:46:43 +0200
> >> 
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> > Martin will correct me if I'm wrong, but I think the current code
> >> > already does what you want: it only changes FrameCols/Rows when the
> >> > terminal was really resized.
> >> 
> >> It obviously does not. I mentioned what happens when making child frames
> >> in adjust_frame_size in my case. There is no SIGWINCH involved. Which
> >> makes me suspect that you are not taking adjust_frame_size into
> >> account?
> >
> > I wasn't talking about child frames, I was talking about "normal"
> > frames.
> 
> We're taking past each other.

Maybe.  You were criticizing the existing code, which wasn't supposed
to handle child frames on TTYs, so I interpreted that as unrelated to
child frames, like a kind-of general critique of the design and
implementation of frame resizing on TTYs.  And that was the context in
which I replied.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 16:35                     ` Eli Zaretskii
@ 2024-09-02 16:38                       ` Gerd Möllmann
  2024-09-02 17:39                         ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02 16:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel, rudalics

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
>> Date: Mon, 02 Sep 2024 18:24:12 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
>> >> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
>> >> Date: Mon, 02 Sep 2024 17:46:43 +0200
>> >> 
>> >> Eli Zaretskii <eliz@gnu.org> writes:
>> >> 
>> >> > Martin will correct me if I'm wrong, but I think the current code
>> >> > already does what you want: it only changes FrameCols/Rows when the
>> >> > terminal was really resized.
>> >> 
>> >> It obviously does not. I mentioned what happens when making child frames
>> >> in adjust_frame_size in my case. There is no SIGWINCH involved. Which
>> >> makes me suspect that you are not taking adjust_frame_size into
>> >> account?
>> >
>> > I wasn't talking about child frames, I was talking about "normal"
>> > frames.
>> 
>> We're taking past each other.
>
> Maybe.  You were criticizing the existing code, which wasn't supposed
> to handle child frames on TTYs, so I interpreted that as unrelated to
> child frames, like a kind-of general critique of the design and
> implementation of frame resizing on TTYs.  And that was the context in
> which I replied.

And I said I wonder if one couldn't brew a make-frame, a normal frame,
that also leads to the same problem. And you said here's a bug for that.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 16:31                 ` Gerd Möllmann
@ 2024-09-02 17:32                   ` martin rudalics
  2024-09-02 18:02                     ` Gerd Möllmann
  0 siblings, 1 reply; 23+ messages in thread
From: martin rudalics @ 2024-09-02 17:32 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: Eli Zaretskii, emacs-devel

 > I think I have the above here. It reads
 >
 >    /* Assign new sizes.  */
 >    FRAME_COLS (f) = new_text_cols;
 >    FRAME_LINES (f) = new_text_lines;

It might cause something weird in resize_frame_windows, but that's maybe
only here.

 > Anyway. My problem is setting FrameCols/Rows in adjust_frame_size. It's
 > not guaranteed that the terminal's size has indeed changed when that is
 > done.

You mean the

	FrameRows (FRAME_TTY (f)) =
	FrameCols (FRAME_TTY (f)) =

assignments?  These are from the pre-2014 change_frame_size_1 code with
the comments mostly preserved and governed by similar conditions.  Do
you think something changed here?

But I agree that for child frames with their own smaller dimensions
these assignments should be obviously skipped.

martin



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 16:38                       ` Gerd Möllmann
@ 2024-09-02 17:39                         ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2024-09-02 17:39 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: emacs-devel, rudalics

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org,  rudalics@gmx.at
> Date: Mon, 02 Sep 2024 18:38:58 +0200
> 
> >> We're taking past each other.
> >
> > Maybe.  You were criticizing the existing code, which wasn't supposed
> > to handle child frames on TTYs, so I interpreted that as unrelated to
> > child frames, like a kind-of general critique of the design and
> > implementation of frame resizing on TTYs.  And that was the context in
> > which I replied.
> 
> And I said I wonder if one couldn't brew a make-frame, a normal frame,
> that also leads to the same problem. And you said here's a bug for that.

That bug was fixed.  And it was triggered by SIGWINCH, not by other
calls that resize the frame.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 17:32                   ` martin rudalics
@ 2024-09-02 18:02                     ` Gerd Möllmann
  2024-09-02 18:26                       ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Gerd Möllmann @ 2024-09-02 18:02 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, emacs-devel

martin rudalics <rudalics@gmx.at> writes:

>> I think I have the above here. It reads
>>
>>    /* Assign new sizes.  */
>>    FRAME_COLS (f) = new_text_cols;
>>    FRAME_LINES (f) = new_text_lines;
>
> It might cause something weird in resize_frame_windows, but that's maybe
> only here.
>
>> Anyway. My problem is setting FrameCols/Rows in adjust_frame_size. It's
>> not guaranteed that the terminal's size has indeed changed when that is
>> done.
>
> You mean the
>
> 	FrameRows (FRAME_TTY (f)) =
> 	FrameCols (FRAME_TTY (f)) =
>
> assignments?

Exactly.

> These are from the pre-2014 change_frame_size_1 code with the comments
> mostly preserved and governed by similar conditions. Do you think
> something changed here?

Actually, I wondered if this might be something I overlooked when writing
the new redisplay. Last century :-). Alas, there were so many changes in
the meantime. that I give up trying to find out.

> But I agree that for child frames with their own smaller dimensions
> these assignments should be obviously skipped.

Yes, for child frames it's certainly wrong. And I can easily work around
that.

What I wonder is if it's the right thing to do for normal frames, as
well. What happens if I make-frame a second non-child tty frame with
frame parameters that lead to the FrameRow/Col assignments with smaller
values?

If that happens, the terminal appears to have changed size, which it of
course hasn't. And redisplaying the first frame might trigger an
emacs_abort in cmcheckmagic that tries to make sure that we don't write
beyond the terminal borders, which have been set by the new fraem.

And more general, I wonder why setting the physical terminal size in
adjust_frame_size is the right thing to begin with. I believe
determining the terminal size should happen elsewhere and the size is
and input to the algorith resizing frames.

I don't know whqt Eli thinks about this, as you might have noticed :-)



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Question about dubious code for terminal frames
  2024-09-02 18:02                     ` Gerd Möllmann
@ 2024-09-02 18:26                       ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2024-09-02 18:26 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: rudalics, emacs-devel

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Mon, 02 Sep 2024 20:02:24 +0200
> 
> What I wonder is if it's the right thing to do for normal frames, as
> well. What happens if I make-frame a second non-child tty frame with
> frame parameters that lead to the FrameRow/Col assignments with smaller
> values?

According to my testing, nothing happens: FrameRow/Col are set to
their actual values derived from the actual dimensions of the
terminal.



^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2024-09-02 18:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-02  9:04 Question about dubious code for terminal frames Gerd Möllmann
2024-09-02  9:10 ` Andreas Schwab
2024-09-02  9:48   ` Gerd Möllmann
2024-09-02 10:23     ` Andreas Schwab
2024-09-02 10:46       ` Gerd Möllmann
2024-09-02 11:44 ` Eli Zaretskii
2024-09-02 12:59   ` Gerd Möllmann
2024-09-02 13:17     ` Eli Zaretskii
2024-09-02 14:11       ` Gerd Möllmann
2024-09-02 14:35         ` Eli Zaretskii
2024-09-02 14:54           ` Gerd Möllmann
2024-09-02 15:31             ` Eli Zaretskii
2024-09-02 15:46               ` Gerd Möllmann
2024-09-02 15:55                 ` Eli Zaretskii
2024-09-02 16:24                   ` Gerd Möllmann
2024-09-02 16:35                     ` Eli Zaretskii
2024-09-02 16:38                       ` Gerd Möllmann
2024-09-02 17:39                         ` Eli Zaretskii
2024-09-02 16:20               ` martin rudalics
2024-09-02 16:31                 ` Gerd Möllmann
2024-09-02 17:32                   ` martin rudalics
2024-09-02 18:02                     ` Gerd Möllmann
2024-09-02 18:26                       ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).