[PATCH/RFA] Sync window sizes between Readline and GDB

[PATCH/RFA] Sync window sizes between Readline and GDB

[Mark Kettenis]

Folks,

The attached makes sure GDB's and Readline's idea of the screen size
are the same.  It gets rid of some rather platform-specific code and
uses similar code in Readline instead.  This fixes the problems I was
seeing in completion.exp.

I don't think we should check this in on the branch, since there is a
certain risk involved that things break for certain hosts.  However, I
think making use of the code in Readline is a good thing since that
code is defenitely better tested than our code.

OK for mainline?

Mark

Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* utils.c (set_width_command, set_width): Remove prototypes.
	(set_screen_size): New prototype.
	(init_page_info): Simplify by fetching the screen size from
	Readline.  Call set_screen_size.
	(set_screen_size): New function.
	(set_width): Add missing whitespace in comment.
	(set_width_command): Call set_screen_size.
	(set_height_command): New function.
	(initialize_utils): Fix formatting.  Make "set height" command
	call set_height_command.  Remove redundant code that turns off
	pagination if output isn't a terminal.  Remove redundant call to
	set_width_command.

Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.105
diff -u -p -r1.105 utils.c
--- utils.c 9 Aug 2003 14:57:30 -0000 1.105
+++ utils.c 17 Aug 2003 18:31:04 -0000
@@ -102,10 +102,6 @@ static void malloc_botch (void);
 
 static void prompt_for_continue (void);
 
-static void set_width_command (char *, int, struct cmd_list_element *);
-
-static void set_width (void);
-
 /* Chain of cleanup actions established with make_cleanup,
    to be executed if an error happens.  */
 
@@ -1567,11 +1563,12 @@ fputstrn_unfiltered (const char *str, in
 }
 \f
 
-
 /* Number of lines per page or UINT_MAX if paging is disabled.  */
 static unsigned int lines_per_page;
+
 /* Number of chars per line or UINT_MAX if line folding is disabled.  */
 static unsigned int chars_per_line;
+
 /* Current count of lines printed on this page, chars on this line.  */
 static unsigned int lines_printed, chars_printed;
 
@@ -1600,7 +1597,10 @@ static char *wrap_indent;
 static int wrap_column;
 \f
 
-/* Inialize the lines and chars per page */
+/* Inialize the number of lines per page and chars per line.  */
+
+static void set_screen_size (void);
+
 void
 init_page_info (void)
 {
@@ -1608,66 +1608,60 @@ init_page_info (void)
   if (!tui_get_command_dimension (&chars_per_line, &lines_per_page))
 #endif
     {
-      /* These defaults will be used if we are unable to get the correct
-         values from termcap.  */
-#if defined(__GO32__)
-      lines_per_page = ScreenRows ();
-      chars_per_line = ScreenCols ();
-#else
-      lines_per_page = 24;
-      chars_per_line = 80;
-
-#if !defined (_WIN32)
-      /* Initialize the screen height and width from termcap.  */
-      {
-	char *termtype = getenv ("TERM");
+      int rows, cols;
 
-	/* Positive means success, nonpositive means failure.  */
-	int status;
+      /* Make sure Readline has initialized its terminal settings.  */
+      rl_reset_terminal (NULL);
 
-	/* 2048 is large enough for all known terminals, according to the
-	   GNU termcap manual.  */
-	char term_buffer[2048];
+      /* Get the screen size from Readline.  */
+      rl_get_screen_size (&rows, &cols);
+      lines_per_page = rows;
+      chars_per_line = cols;
 
-	if (termtype)
-	  {
-	    status = tgetent (term_buffer, termtype);
-	    if (status > 0)
-	      {
-		int val;
-		int running_in_emacs = getenv ("EMACS") != NULL;
-
-		val = tgetnum ("li");
-		if (val >= 0 && !running_in_emacs)
-		  lines_per_page = val;
-		else
-		  /* The number of lines per page is not mentioned
-		     in the terminal description.  This probably means
-		     that paging is not useful (e.g. emacs shell window),
-		     so disable paging.  */
-		  lines_per_page = UINT_MAX;
-
-		val = tgetnum ("co");
-		if (val >= 0)
-		  chars_per_line = val;
-	      }
-	  }
-      }
-#endif
+      /* Readline should have fetched the termcap entry for us.  */
+      if (tgetnum ("li") < 0 || getenv ("EMACS"))
+	{
+	  /* The number of lines per page is not mentioned in the
+	     terminal description.  This probably means that paging is
+	     not useful (e.g. emacs shell window), so disable paging.  */
+	  lines_per_page = UINT_MAX;
+	}
 
+      /* FIXME: Get rid of this junk.  */
 #if defined(SIGWINCH) && defined(SIGWINCH_HANDLER)
-
-      /* If there is a better way to determine the window size, use it. */
       SIGWINCH_HANDLER (SIGWINCH);
 #endif
-#endif
+
       /* If the output is not a terminal, don't paginate it.  */
       if (!ui_file_isatty (gdb_stdout))
 	lines_per_page = UINT_MAX;
-    }				/* the command_line_version */
+    }
+
+  set_screen_size ();
   set_width ();
 }
 
+/* Set the screen size based on LINES_PER_PAGE and CHARS_PER_LINE.  */
+
+static void
+set_screen_size (void)
+{
+  int rows = lines_per_page;
+  int cols = chars_per_line;
+
+  if (rows <= 0)
+    rows = INT_MAX;
+
+  if (cols <= 0)
+    rl_get_screen_size (NULL, &cols);
+
+  /* Update Readline's idea of the terminal size.  */
+  rl_set_screen_size (rows, cols);
+}
+
+/* Reinitialize WRAP_BUFFER according to the current value of
+   CHARS_PER_LINE.  */
+
 static void
 set_width (void)
 {
@@ -1681,16 +1675,24 @@ set_width (void)
     }
   else
     wrap_buffer = (char *) xrealloc (wrap_buffer, chars_per_line + 2);
-  wrap_pointer = wrap_buffer;	/* Start it at the beginning */
+  wrap_pointer = wrap_buffer;	/* Start it at the beginning.  */
 }
 
 /* ARGSUSED */
 static void
 set_width_command (char *args, int from_tty, struct cmd_list_element *c)
 {
+  set_screen_size ();
   set_width ();
 }
 
+/* ARGSUSED */
+static void
+set_height_command (char *args, int from_tty, struct cmd_list_element *c)
+{
+  set_screen_size ();
+}
+
 /* Wait, so the user can read what's on the screen.  Prompt the user
    to continue by pressing RETURN.  */
 
@@ -2486,26 +2488,18 @@ initialize_utils (void)
 {
   struct cmd_list_element *c;
 
-  c = add_set_cmd ("width", class_support, var_uinteger,
-		   (char *) &chars_per_line,
+  c = add_set_cmd ("width", class_support, var_uinteger, &chars_per_line,
 		   "Set number of characters gdb thinks are in a line.",
 		   &setlist);
   add_show_from_set (c, &showlist);
   set_cmd_sfunc (c, set_width_command);
 
-  add_show_from_set
-    (add_set_cmd ("height", class_support,
-		  var_uinteger, (char *) &lines_per_page,
-		  "Set number of lines gdb thinks are in a page.", &setlist),
-     &showlist);
+  c = add_set_cmd ("height", class_support, var_uinteger, &lines_per_page,
+		   "Set number of lines gdb thinks are in a page.", &setlist);
+  add_show_from_set (c, &showlist);
+  set_cmd_sfunc (c, set_height_command);
 
   init_page_info ();
-
-  /* If the output is not a terminal, don't paginate it.  */
-  if (!ui_file_isatty (gdb_stdout))
-    lines_per_page = UINT_MAX;
-
-  set_width_command ((char *) NULL, 0, c);
 
   add_show_from_set
     (add_set_cmd ("demangle", class_support, var_boolean,

Re: [PATCH/RFA] Sync window sizes between Readline and GDB

[Daniel Jacobowitz]

On Sun, Aug 17, 2003 at 08:41:13PM +0200, Mark Kettenis wrote:
> Folks,
> 
> The attached makes sure GDB's and Readline's idea of the screen size
> are the same.  It gets rid of some rather platform-specific code and
> uses similar code in Readline instead.  This fixes the problems I was
> seeing in completion.exp.
> 
> I don't think we should check this in on the branch, since there is a
> certain risk involved that things break for certain hosts.  However, I
> think making use of the code in Readline is a good thing since that
> code is defenitely better tested than our code.
> 
> OK for mainline?

This certainly looks like a good idea to me.  I tried it on a GNU/Linux
system that was showing similar problems, and it fixed those as well.

It introduces new warnings in utils.c though.  There's one call to
set_width above its declaration and you removed the prototype.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH/RFA] Sync window sizes between Readline and GDB

[Eli Zaretskii]

> Date: Sun, 17 Aug 2003 20:41:13 +0200 (CEST)
> From: Mark Kettenis <kettenis@chello.nl>
> 
> The attached makes sure GDB's and Readline's idea of the screen size
> are the same.  It gets rid of some rather platform-specific code and
> uses similar code in Readline instead.

For the DJGPP port, you are introducing calls that were not made
before.  I'm not sure they will work, especially the call to tgetnum
(DJGPP doesn't have a termcap/terminfo emulation yet).

Re: [PATCH/RFA] Sync window sizes between Readline and GDB

[Mark Kettenis]

   Date: Mon, 18 Aug 2003 12:48:42 +0200
   From: "Eli Zaretskii" <eliz@elta.co.il>

   > Date: Sun, 17 Aug 2003 20:41:13 +0200 (CEST)
   > From: Mark Kettenis <kettenis@chello.nl>
   > 
   > The attached makes sure GDB's and Readline's idea of the screen size
   > are the same.  It gets rid of some rather platform-specific code and
   > uses similar code in Readline instead.

   For the DJGPP port, you are introducing calls that were not made
   before.  I'm not sure they will work, especially the call to tgetnum
   (DJGPP doesn't have a termcap/terminfo emulation yet).

Ah sorry, I was under the impression you had some sort of termcap
emulation.  Anyway, I suppose you don't have this terminal madness on
plain old DOS.  I'll re-instate the old code before checking this in.

Thanks,

Mark

Re: [PATCH/RFA] Sync window sizes between Readline and GDB

[Mark Kettenis]

   Date: Mon, 18 Aug 2003 12:48:42 +0200
   From: "Eli Zaretskii" <eliz@elta.co.il>

   > Date: Sun, 17 Aug 2003 20:41:13 +0200 (CEST)
   > From: Mark Kettenis <kettenis@chello.nl>
   > 
   > The attached makes sure GDB's and Readline's idea of the screen size
   > are the same.  It gets rid of some rather platform-specific code and
   > uses similar code in Readline instead.

   For the DJGPP port, you are introducing calls that were not made
   before.  I'm not sure they will work, especially the call to tgetnum
   (DJGPP doesn't have a termcap/terminfo emulation yet).

So I left the DJGPP code in place.  Below is what I actually checked
in.

Mark

Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* utils.c (set_width_command): Remove prototypes.
	(set_screen_size): New prototype.
	(init_page_info): Simplify by fetching the screen size from
	Readline.  Call set_screen_size.
	(set_screen_size): New function.
	(set_width): Add missing whitespace in comment.
	(set_width_command): Call set_screen_size.
	(set_height_command): New function.
	(initialize_utils): Fix formatting.  Make "set height" command
	call set_height_command.  Remove redundant code that turns off
	pagination if output isn't a terminal.  Remove redundant call to
	set_width_command.

Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.105
diff -u -p -r1.105 utils.c
--- utils.c 9 Aug 2003 14:57:30 -0000 1.105
+++ utils.c 22 Aug 2003 20:00:05 -0000
@@ -102,8 +102,7 @@ static void malloc_botch (void);
 
 static void prompt_for_continue (void);
 
-static void set_width_command (char *, int, struct cmd_list_element *);
-
+static void set_screen_size (void);
 static void set_width (void);
 
 /* Chain of cleanup actions established with make_cleanup,
@@ -1567,11 +1566,12 @@ fputstrn_unfiltered (const char *str, in
 }
 \f
 
-
 /* Number of lines per page or UINT_MAX if paging is disabled.  */
 static unsigned int lines_per_page;
+
 /* Number of chars per line or UINT_MAX if line folding is disabled.  */
 static unsigned int chars_per_line;
+
 /* Current count of lines printed on this page, chars on this line.  */
 static unsigned int lines_printed, chars_printed;
 
@@ -1600,7 +1600,8 @@ static char *wrap_indent;
 static int wrap_column;
 \f
 
-/* Inialize the lines and chars per page */
+/* Inialize the number of lines per page and chars per line.  */
+
 void
 init_page_info (void)
 {
@@ -1608,66 +1609,65 @@ init_page_info (void)
   if (!tui_get_command_dimension (&chars_per_line, &lines_per_page))
 #endif
     {
-      /* These defaults will be used if we are unable to get the correct
-         values from termcap.  */
 #if defined(__GO32__)
       lines_per_page = ScreenRows ();
       chars_per_line = ScreenCols ();
 #else
-      lines_per_page = 24;
-      chars_per_line = 80;
+      int rows, cols;
 
-#if !defined (_WIN32)
-      /* Initialize the screen height and width from termcap.  */
-      {
-	char *termtype = getenv ("TERM");
-
-	/* Positive means success, nonpositive means failure.  */
-	int status;
+      /* Make sure Readline has initialized its terminal settings.  */
+      rl_reset_terminal (NULL);
 
-	/* 2048 is large enough for all known terminals, according to the
-	   GNU termcap manual.  */
-	char term_buffer[2048];
+      /* Get the screen size from Readline.  */
+      rl_get_screen_size (&rows, &cols);
+      lines_per_page = rows;
+      chars_per_line = cols;
 
-	if (termtype)
-	  {
-	    status = tgetent (term_buffer, termtype);
-	    if (status > 0)
-	      {
-		int val;
-		int running_in_emacs = getenv ("EMACS") != NULL;
-
-		val = tgetnum ("li");
-		if (val >= 0 && !running_in_emacs)
-		  lines_per_page = val;
-		else
-		  /* The number of lines per page is not mentioned
-		     in the terminal description.  This probably means
-		     that paging is not useful (e.g. emacs shell window),
-		     so disable paging.  */
-		  lines_per_page = UINT_MAX;
-
-		val = tgetnum ("co");
-		if (val >= 0)
-		  chars_per_line = val;
-	      }
-	  }
-      }
-#endif
+      /* Readline should have fetched the termcap entry for us.  */
+      if (tgetnum ("li") < 0 || getenv ("EMACS"))
+	{
+	  /* The number of lines per page is not mentioned in the
+	     terminal description.  This probably means that paging is
+	     not useful (e.g. emacs shell window), so disable paging.  */
+	  lines_per_page = UINT_MAX;
+	}
 
+      /* FIXME: Get rid of this junk.  */
 #if defined(SIGWINCH) && defined(SIGWINCH_HANDLER)
-
-      /* If there is a better way to determine the window size, use it. */
       SIGWINCH_HANDLER (SIGWINCH);
 #endif
-#endif
+
       /* If the output is not a terminal, don't paginate it.  */
       if (!ui_file_isatty (gdb_stdout))
 	lines_per_page = UINT_MAX;
-    }				/* the command_line_version */
+    }
+#endif
+
+  set_screen_size ();
   set_width ();
 }
 
+/* Set the screen size based on LINES_PER_PAGE and CHARS_PER_LINE.  */
+
+static void
+set_screen_size (void)
+{
+  int rows = lines_per_page;
+  int cols = chars_per_line;
+
+  if (rows <= 0)
+    rows = INT_MAX;
+
+  if (cols <= 0)
+    rl_get_screen_size (NULL, &cols);
+
+  /* Update Readline's idea of the terminal size.  */
+  rl_set_screen_size (rows, cols);
+}
+
+/* Reinitialize WRAP_BUFFER according to the current value of
+   CHARS_PER_LINE.  */
+
 static void
 set_width (void)
 {
@@ -1681,16 +1681,24 @@ set_width (void)
     }
   else
     wrap_buffer = (char *) xrealloc (wrap_buffer, chars_per_line + 2);
-  wrap_pointer = wrap_buffer;	/* Start it at the beginning */
+  wrap_pointer = wrap_buffer;	/* Start it at the beginning.  */
 }
 
 /* ARGSUSED */
 static void
 set_width_command (char *args, int from_tty, struct cmd_list_element *c)
 {
+  set_screen_size ();
   set_width ();
 }
 
+/* ARGSUSED */
+static void
+set_height_command (char *args, int from_tty, struct cmd_list_element *c)
+{
+  set_screen_size ();
+}
+
 /* Wait, so the user can read what's on the screen.  Prompt the user
    to continue by pressing RETURN.  */
 
@@ -2486,26 +2494,18 @@ initialize_utils (void)
 {
   struct cmd_list_element *c;
 
-  c = add_set_cmd ("width", class_support, var_uinteger,
-		   (char *) &chars_per_line,
+  c = add_set_cmd ("width", class_support, var_uinteger, &chars_per_line,
 		   "Set number of characters gdb thinks are in a line.",
 		   &setlist);
   add_show_from_set (c, &showlist);
   set_cmd_sfunc (c, set_width_command);
 
-  add_show_from_set
-    (add_set_cmd ("height", class_support,
-		  var_uinteger, (char *) &lines_per_page,
-		  "Set number of lines gdb thinks are in a page.", &setlist),
-     &showlist);
+  c = add_set_cmd ("height", class_support, var_uinteger, &lines_per_page,
+		   "Set number of lines gdb thinks are in a page.", &setlist);
+  add_show_from_set (c, &showlist);
+  set_cmd_sfunc (c, set_height_command);
 
   init_page_info ();
-
-  /* If the output is not a terminal, don't paginate it.  */
-  if (!ui_file_isatty (gdb_stdout))
-    lines_per_page = UINT_MAX;
-
-  set_width_command ((char *) NULL, 0, c);
 
   add_show_from_set
     (add_set_cmd ("demangle", class_support, var_boolean,