[patch] Correct origin calculation of command line window in TUI mode

[patch] Correct origin calculation of command line window in TUI mode

[Balazs Kezes]

The locator's (a small bar between source and command window) origin (the first
row's index) is not updated in tui_resize_all and this outdated data is used to
calculate the command window's origin. This patch fixes this by updating it.

--
Balazs

Index: tui-win.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-win.c,v
retrieving revision 1.47
diff -c -p -r1.47 tui-win.c
*** tui-win.c	17 May 2010 22:21:43 -0000	1.47
--- tui-win.c	21 Jul 2010 20:56:49 -0000
*************** tui_resize_all (void)
*** 712,717 ****
--- 712,718 ----
  	  else
  	    new_height = first_win->generic.height + split_diff;

+ 	  locator->origin.y = new_height + 1;
  	  make_invisible_and_set_new_height (first_win, new_height);
  	  TUI_CMD_WIN->generic.origin.y = locator->origin.y + 1;
  	  TUI_CMD_WIN->generic.width += width_diff;

Re: [patch] Correct origin calculation of command line window in TUI mode

[Pedro Alves]

On Wednesday 21 July 2010 22:02:13, Balazs Kezes wrote:
> The locator's (a small bar between source and command window) origin (the first
> row's index) is not updated in tui_resize_all and this outdated data is used to
> calculate the command window's origin. This patch fixes this by updating it.

Hmmm, I've tried all three patches together, and I still get a messed
up cmd window after a couple of resizes (e.g., start
maxized/unmaximize/maximize).  With just the "call tui_resize_all"
patch applied, and the one below in addition, I get somewhat better
result than with them all applied.  Do you see something like that too?

Makes me wonder where's the code that calculates all the initial
sub window sizes for first display, and whether we could make more reuse
out of it.

> 
> --
> Balazs
> 
> Index: tui-win.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/tui/tui-win.c,v
> retrieving revision 1.47
> diff -c -p -r1.47 tui-win.c
> *** tui-win.c	17 May 2010 22:21:43 -0000	1.47
> --- tui-win.c	21 Jul 2010 20:56:49 -0000
> *************** tui_resize_all (void)
> *** 712,717 ****
> --- 712,718 ----
>   	  else
>   	    new_height = first_win->generic.height + split_diff;
> 
> + 	  locator->origin.y = new_height + 1;
>   	  make_invisible_and_set_new_height (first_win, new_height);
>   	  TUI_CMD_WIN->generic.origin.y = locator->origin.y + 1;
>   	  TUI_CMD_WIN->generic.width += width_diff;
> 


-- 
Pedro Alves

Re: [patch] Correct origin calculation of command line window in TUI mode

[Balazs Kezes]

> Hmmm, I've tried all three patches together, and I still get a messed
> up cmd window after a couple of resizes (e.g., start
> maxized/unmaximize/maximize).

Yeah, these 3 patches are not enough. There are two remaining modifications
which need to be done:

Yeah, these 3 patches are not enough. There are two remaining modifications
which need to be done:
1) HAVE_RESIZE_TERM - to make sure ncurses knows about the terminal size.
   Somehow this needs to be added to the configuration. I'm not sure how to do
   this. Maybe I'll look into this sometime.
2) The readline modification in order to have correct sizes. I've already
   contacted the readline guys and they told me that this bug will be fixed in
   readline 6.2.

Even after this there is a little annoyance that you need to press a
key to actually resize and redraw everything, but this could be fixed
by ungetc or with a similar hack.

Here are the changelogs:

This patch:

2010-07-26 Balazs Kezes <rlblaster@gmail.com>

	* tui-win.c (tui_resize_all): Update the locator's origin's
	coordinates.

The resizing patch:

2010-07-26 Balazs Kezes <rlblaster@gmail.com>

	* tui-win.c (make_visible_with_new_height): Resize and move
	the command window with to the new size and position.

The tui_resize_all patch:

2010-07-26 Balazs Kezes <rlblaster@gmail.com>

	* tui-io.c (tui_handle_resize_during_io): Call tui_resize_all after a
	detecting a resize.
	* tui-win.c (tui_resize_all): Remove tui_set_win_resized_to (TRUE);
	because this function will be called when this is already true.


--
Balazs


On 7/27/10, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 21 July 2010 22:02:13, Balazs Kezes wrote:
>> The locator's (a small bar between source and command window) origin (the
>> first
>> row's index) is not updated in tui_resize_all and this outdated data is
>> used to
>> calculate the command window's origin. This patch fixes this by updating
>> it.
>
> Hmmm, I've tried all three patches together, and I still get a messed
> up cmd window after a couple of resizes (e.g., start
> maxized/unmaximize/maximize).  With just the "call tui_resize_all"
> patch applied, and the one below in addition, I get somewhat better
> result than with them all applied.  Do you see something like that too?
>
> Makes me wonder where's the code that calculates all the initial
> sub window sizes for first display, and whether we could make more reuse
> out of it.
>
>>
>> --
>> Balazs
>>
>> Index: tui-win.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/tui/tui-win.c,v
>> retrieving revision 1.47
>> diff -c -p -r1.47 tui-win.c
>> *** tui-win.c	17 May 2010 22:21:43 -0000	1.47
>> --- tui-win.c	21 Jul 2010 20:56:49 -0000
>> *************** tui_resize_all (void)
>> *** 712,717 ****
>> --- 712,718 ----
>>   	  else
>>   	    new_height = first_win->generic.height + split_diff;
>>
>> + 	  locator->origin.y = new_height + 1;
>>   	  make_invisible_and_set_new_height (first_win, new_height);
>>   	  TUI_CMD_WIN->generic.origin.y = locator->origin.y + 1;
>>   	  TUI_CMD_WIN->generic.width += width_diff;
>>
>
>
> --
> Pedro Alves
>

Re: [patch] Correct origin calculation of command line window in TUI mode

[Pedro Alves]

On Tuesday 27 July 2010 23:38:02, Balazs Kezes wrote:
> > Hmmm, I've tried all three patches together, and I still get a messed
> > up cmd window after a couple of resizes (e.g., start
> > maxized/unmaximize/maximize).
> 
> Yeah, these 3 patches are not enough. There are two remaining modifications
> which need to be done:

Ah.

> 1) HAVE_RESIZE_TERM - to make sure ncurses knows about the terminal size.
>    Somehow this needs to be added to the configuration. I'm not sure how to do
>    this. Maybe I'll look into this sometime.

I'll do this.  These three patches now put you at borderline of
what we can accept without copyright assignment to the FSF.  Do
you have that taken care of?  If not, let us know, and we'll start
you on the process.

> 2) The readline modification in order to have correct sizes. I've already
>    contacted the readline guys and they told me that this bug will be fixed in
>    readline 6.2.

That's good news.  In that case, I don't think there's a problem in
backporting the fix to our local copy.  Distros can do the same to
their system readline's if they care too.

> Even after this there is a little annoyance that you need to press a
> key to actually resize and redraw everything, but this could be fixed
> by ungetc or with a similar hack.
> 
> Here are the changelogs:

Thanks.  I'll take another look at this soon.

-- 
Pedro Alves

Re: [patch] Correct origin calculation of command line window in TUI mode

[Balazs Kezes]

> These three patches now put you at borderline of
> what we can accept without copyright assignment to the FSF.  Do
> you have that taken care of?  If not, let us know, and we'll start
> you on the process.

I'm not sure what you are talking about. I don't think I'll be doing a lot
more work on this so it might be superfluous if this means what I think.


On 7/28/10, Pedro Alves <pedro@codesourcery.com> wrote:
> On Tuesday 27 July 2010 23:38:02, Balazs Kezes wrote:
>> > Hmmm, I've tried all three patches together, and I still get a messed
>> > up cmd window after a couple of resizes (e.g., start
>> > maxized/unmaximize/maximize).
>>
>> Yeah, these 3 patches are not enough. There are two remaining
>> modifications
>> which need to be done:
>
> Ah.
>
>> 1) HAVE_RESIZE_TERM - to make sure ncurses knows about the terminal size.
>>    Somehow this needs to be added to the configuration. I'm not sure how
>> to do
>>    this. Maybe I'll look into this sometime.
>
> I'll do this.  These three patches now put you at borderline of
> what we can accept without copyright assignment to the FSF.  Do
> you have that taken care of?  If not, let us know, and we'll start
> you on the process.
>
>> 2) The readline modification in order to have correct sizes. I've already
>>    contacted the readline guys and they told me that this bug will be
>> fixed in
>>    readline 6.2.
>
> That's good news.  In that case, I don't think there's a problem in
> backporting the fix to our local copy.  Distros can do the same to
> their system readline's if they care too.
>
>> Even after this there is a little annoyance that you need to press a
>> key to actually resize and redraw everything, but this could be fixed
>> by ungetc or with a similar hack.
>>
>> Here are the changelogs:
>
> Thanks.  I'll take another look at this soon.
>
> --
> Pedro Alves
>

Re: [patch] Correct origin calculation of command line window in TUI mode

[Pedro Alves]

On Wednesday 28 July 2010 08:24:15, Balazs Kezes wrote:
> > These three patches now put you at borderline of
> > what we can accept without copyright assignment to the FSF.  Do
> > you have that taken care of?  If not, let us know, and we'll start
> > you on the process.
> 
> I'm not sure what you are talking about. 

In a nutshell, before we can accept code contributions, we need a
copyright assignment form filled out and filed with the FSF.  Small
changes (around 15 lines or code, say) can be accepted without a
copyright assignment form on file.

Please see the section about copyright assignment in src/gdb/CONTRIBUTE,
and <http://www.gnu.org/prep/maintain.html#SEC6>.

I hope this does not demotivate you from contributing.

> I don't think I'll be doing a lot more work on this so it might 
> be superfluous if this means what I think.

I'm sure you'll find other itches to scratch in gdb.  :-)

-- 
Pedro Alves

Re: [patch] Correct origin calculation of command line window in TUI mode

[Pedro Alves]

On Tuesday 27 July 2010 23:46:46, Pedro Alves wrote:
> > On Tuesday 27 July 2010 23:38:02, Balazs Kezes wrote:
> > Yeah, these 3 patches are not enough. There are two remaining modifications
> > which need to be done:
> 
> > 1) HAVE_RESIZE_TERM - to make sure ncurses knows about the terminal size.
> >    Somehow this needs to be added to the configuration. I'm not sure how to do
> >    this. Maybe I'll look into this sometime.
> 
> I'll do this.  (...)

I've now applied this patch below.  Thanks.

-- 
Pedro Alves

2010-07-28  Pedro Alves  <pedro@codesourcery.com>

	* configure.ac: Check for resize_term.
	* configure, config.in: Regenerate.

---
 gdb/config.in    |    3 +++
 gdb/configure    |    2 +-
 gdb/configure.ac |    2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

Index: src/gdb/configure.ac
===================================================================
--- src.orig/gdb/configure.ac	2010-07-28 12:39:02.000000000 +0100
+++ src/gdb/configure.ac	2010-07-28 12:41:35.000000000 +0100
@@ -954,7 +954,7 @@ AC_FUNC_ALLOCA
 AC_FUNC_MMAP
 AC_FUNC_VFORK
 AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid \
-		getgid pipe poll pread64 sbrk setpgid setpgrp setsid \
+		getgid pipe poll pread64 resize_term sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
 		ttrace wborder setlocale iconvlist libiconvlist btowc \
 		setrlimit getrlimit posix_madvise])
Index: src/gdb/configure
===================================================================
--- src.orig/gdb/configure	2010-07-28 12:39:02.000000000 +0100
+++ src/gdb/configure	2010-07-28 12:41:35.000000000 +0100
@@ -12509,7 +12509,7 @@ $as_echo "#define HAVE_WORKING_FORK 1" >
 fi
 
 for ac_func in canonicalize_file_name realpath getrusage getuid \
-		getgid pipe poll pread64 sbrk setpgid setpgrp setsid \
+		getgid pipe poll pread64 resize_term sbrk setpgid setpgrp setsid \
 		sigaction sigprocmask sigsetmask socketpair syscall \
 		ttrace wborder setlocale iconvlist libiconvlist btowc \
 		setrlimit getrlimit posix_madvise
Index: src/gdb/config.in
===================================================================
--- src.orig/gdb/config.in	2010-07-28 12:39:02.000000000 +0100
+++ src/gdb/config.in	2010-07-28 12:41:35.000000000 +0100
@@ -461,6 +461,9 @@
 /* Define to 1 if you have the `realpath' function. */
 #undef HAVE_REALPATH
 
+/* Define to 1 if you have the `resize_term' function. */
+#undef HAVE_RESIZE_TERM
+
 /* Define to 1 if you have the `sbrk' function. */
 #undef HAVE_SBRK
 

Re: [patch] Correct origin calculation of command line window in TUI mode

[Pedro Alves]

Applied, thanks!

-- 
Pedro Alves

2010-07-28  Balazs Kezes  <rlblaster@gmail.com>

	* tui/tui-win.c (tui_resize_all): Update the locator's origin's
	coordinates.

---
 gdb/tui/tui-win.c |    1 +
 1 file changed, 1 insertion(+)

Index: src/gdb/tui/tui-win.c
===================================================================
--- src.orig/gdb/tui/tui-win.c	2010-07-27 15:30:34.000000000 +0100
+++ src/gdb/tui/tui-win.c	2010-07-27 15:34:34.000000000 +0100
@@ -712,6 +712,7 @@ tui_resize_all (void)
 	  else
 	    new_height = first_win->generic.height + split_diff;
 
+	  locator->origin.y = new_height + 1;
 	  make_invisible_and_set_new_height (first_win, new_height);
 	  TUI_CMD_WIN->generic.origin.y = locator->origin.y + 1;
 	  TUI_CMD_WIN->generic.width += width_diff;