public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCHSET] [2/4] Fix various issue in TUI
@ 2014-12-31 17:45 Eli Zaretskii
  2015-01-05 19:11 ` Pedro Alves
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2014-12-31 17:45 UTC (permalink / raw)
  To: gdb-patches

This patch fixes the problem whereby setting the border attributes had
no effect whatsoever.  Turns out no one was calling the functions that
detected changes in these settings and applied them to windows.

While at that, this also fixes a copy/paste error in a comment to a
function.

OK?

2014-12-31  Eli Zaretskii  <eliz@gnu.org>

	* tui/tui-win.c (tui_rehighlight_all): New function.

	* tui/tui-win.h: Add prototype for tui_rehighlight_all.

	* tui/tui-command.c (tui_dispatch_ctrl_char): When Ctrl-L was
          pressed, call tui_rehighlight_all if tui_update_variables
          indicates it should be.

	* tui/tui-win.c (tui_refresh_all_command): Call
          tui_rehighlight_all if tui_update_variables indicates it
          should be.

	* tui/tui-hooks.c (tui_note_setting_change): New function.
	(tui_install_hooks): Install an observer for "command
	parameter changed".
	(tui_remove_hooks): Uninstall the observer.


--- gdb/tui/tui-win.h~0	2014-06-11 18:34:41 +0300
+++ gdb/tui/tui-win.h	2014-12-31 13:38:18 +0200
@@ -35,6 +35,7 @@
 extern void tui_set_win_focus_to (struct tui_win_info *);
 extern void tui_resize_all (void);
 extern void tui_refresh_all_win (void);
+extern void tui_rehighlight_all (void);
 
 extern chtype tui_border_ulcorner;
 extern chtype tui_border_urcorner;


--- gdb/tui/tui-command.c~0	2014-10-29 21:45:50 +0200
+++ gdb/tui/tui-command.c	2014-12-31 13:49:43 +0200
@@ -50,7 +50,12 @@
 
   /* Handle the CTRL-L refresh for each window.  */
   if (ch == '\f')
-    tui_refresh_all_win ();
+    {
+      if (tui_update_variables ())
+	tui_rehighlight_all ();
+
+      tui_refresh_all_win ();
+    }
 
   /* If the command window has the logical focus, or no-one does
      assume it is the command window; in this case, pass the character


--- gdb/tui/tui-hooks.c~0	2014-06-11 18:34:41 +0300
+++ gdb/tui/tui-hooks.c	2014-12-31 14:41:11 +0200
@@ -247,12 +247,23 @@
   tui_display_main ();
 }
 
+/* Refresh the display when settings important to us change.  */
+static void
+tui_note_setting_change (const char *param, const char *value)
+{
+  if (tui_active
+      && strncmp (param, "tui ", sizeof ("tui ") - 1) == 0
+      && tui_update_variables ())
+    tui_rehighlight_all ();
+}
+
 /* Observers created when installing TUI hooks.  */
 static struct observer *tui_bp_created_observer;
 static struct observer *tui_bp_deleted_observer;
 static struct observer *tui_bp_modified_observer;
 static struct observer *tui_inferior_exit_observer;
 static struct observer *tui_about_to_proceed_observer;
+static struct observer *tui_setting_changed_observer;
 
 /* Install the TUI specific hooks.  */
 void
@@ -278,6 +289,9 @@
     = observer_attach_about_to_proceed (tui_about_to_proceed);
 
   deprecated_register_changed_hook = tui_register_changed_hook;
+
+  tui_setting_changed_observer
+    = observer_attach_command_param_changed (tui_note_setting_change);
 }
 
 /* Remove the TUI specific hooks.  */
@@ -300,6 +314,8 @@
   tui_inferior_exit_observer = NULL;
   observer_detach_about_to_proceed (tui_about_to_proceed_observer);
   tui_about_to_proceed_observer = NULL;
+  observer_detach_command_param_changed (tui_setting_changed_observer);
+  tui_setting_changed_observer = NULL;
 }
 
 void _initialize_tui_hooks (void);


--- gdb/tui/tui-win.c~0	2014-10-29 21:45:50 +0200
+++ gdb/tui/tui-win.c	2014-12-31 13:55:32 +0200
@@ -649,6 +649,16 @@
 }
 
 
+void
+tui_rehighlight_all (void)
+{
+  enum tui_win_type type;
+
+  for (type = SRC_WIN; type < MAX_MAJOR_WINDOWS; type++)
+    tui_check_and_display_highlight_if_needed (tui_win_list[type]);
+}
+
+
 /* Resize all the windows based on the terminal size.  This function
    gets called from within the readline sinwinch handler.  */
 void
@@ -985,11 +995,14 @@
   /* Make sure the curses mode is enabled.  */
   tui_enable ();
 
+  if (tui_update_variables ())
+    tui_rehighlight_all ();
+
   tui_refresh_all_win ();
 }
 
 
-/* Set the height of the specified window.  */
+/* Set the tab width of the specified window.  */
 static void
 tui_set_tab_width_command (char *arg, int from_tty)
 {


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

* Re: [PATCHSET] [2/4] Fix various issue in TUI
  2014-12-31 17:45 [PATCHSET] [2/4] Fix various issue in TUI Eli Zaretskii
@ 2015-01-05 19:11 ` Pedro Alves
  2015-01-05 19:40   ` Eli Zaretskii
                     ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Pedro Alves @ 2015-01-05 19:11 UTC (permalink / raw)
  To: Eli Zaretskii, gdb-patches

On 12/31/2014 05:45 PM, Eli Zaretskii wrote:
> This patch fixes the problem whereby setting the border attributes had
> no effect whatsoever.  

Rather, you need to switch out of the TUI and back, with c-x,a.

> Turns out no one was calling the functions that
> detected changes in these settings and applied them to windows.

Thanks.

> While at that, this also fixes a copy/paste error in a comment to a
> function.

Could you please push this part separately?

> 
> OK?
> 
> 2014-12-31  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* tui/tui-win.c (tui_rehighlight_all): New function.
> 

No empty lines between ChangeLog entries, please.

> 	* tui/tui-win.h: Add prototype for tui_rehighlight_all.
> 
> 	* tui/tui-command.c (tui_dispatch_ctrl_char): When Ctrl-L was
>           pressed, call tui_rehighlight_all if tui_update_variables
>           indicates it should be.

tabs vs spaces.

> 
> 	* tui/tui-win.c (tui_refresh_all_command): Call
>           tui_rehighlight_all if tui_update_variables indicates it
>           should be.
> 
> 	* tui/tui-hooks.c (tui_note_setting_change): New function.
> 	(tui_install_hooks): Install an observer for "command
> 	parameter changed".
> 	(tui_remove_hooks): Uninstall the observer.
> 
> 
> --- gdb/tui/tui-win.h~0	2014-06-11 18:34:41 +0300
> +++ gdb/tui/tui-win.h	2014-12-31 13:38:18 +0200
> @@ -35,6 +35,7 @@
>  extern void tui_set_win_focus_to (struct tui_win_info *);
>  extern void tui_resize_all (void);
>  extern void tui_refresh_all_win (void);
> +extern void tui_rehighlight_all (void);
>  
>  extern chtype tui_border_ulcorner;
>  extern chtype tui_border_urcorner;
> 
> 
> --- gdb/tui/tui-command.c~0	2014-10-29 21:45:50 +0200
> +++ gdb/tui/tui-command.c	2014-12-31 13:49:43 +0200
> @@ -50,7 +50,12 @@

Seems like you're not using git diff to generate the diffs
for some reason. If using GNU diff, could you make sure
to use the "-p" switch?  It makes review a lot easier.

>
> --- gdb/tui/tui-command.c~0	2014-10-29 21:45:50 +0200
> +++ gdb/tui/tui-command.c	2014-12-31 13:49:43 +0200
> @@ -50,7 +50,12 @@
>
>    /* Handle the CTRL-L refresh for each window.  */
>    if (ch == '\f')
> -    tui_refresh_all_win ();
> +    {
> +      if (tui_update_variables ())
> +	tui_rehighlight_all ();
> +
> +      tui_refresh_all_win ();


tui_refresh_all_win already calls tui_check_and_display_highlight_if_needed,
so why do we need to call tui_rehighlight_all?  Is it the tui_update_variables
call that is really missing?  But why would we need to call
tui_update_variables on CTRL-L, if we already call it when the user
changes the variables?

> +    }
>
>    /* If the command window has the logical focus, or no-one does
>       assume it is the command window; in this case, pass the character
>
>
> --- gdb/tui/tui-hooks.c~0	2014-06-11 18:34:41 +0300
> +++ gdb/tui/tui-hooks.c	2014-12-31 14:41:11 +0200
> @@ -247,12 +247,23 @@
>    tui_display_main ();
>  }
>
> +/* Refresh the display when settings important to us change.  */
> +static void
> +tui_note_setting_change (const char *param, const char *value)
> +{
> +  if (tui_active
> +      && strncmp (param, "tui ", sizeof ("tui ") - 1) == 0
> +      && tui_update_variables ())
> +    tui_rehighlight_all ();
> +}
> +

Please do this from the "set" hook of the relevant commands instead.
IOW, replace NULL below (and in the other commands):

  add_setshow_enum_cmd ("active-border-mode", no_class, tui_border_mode_enums,
			&tui_active_border_mode, _("\
...
			NULL,
			show_tui_active_border_mode,
			&tui_setlist, &tui_showlist);

> @@ -985,11 +995,14 @@
>    /* Make sure the curses mode is enabled.  */
>    tui_enable ();
>
> +  if (tui_update_variables ())
> +    tui_rehighlight_all ();
> +
>    tui_refresh_all_win ();
>  }
>

I don't understand this one.  When is it ever necessary?

Thanks,
Pedro Alves

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

* Re: [PATCHSET] [2/4] Fix various issue in TUI
  2015-01-05 19:11 ` Pedro Alves
@ 2015-01-05 19:40   ` Eli Zaretskii
  2015-01-06 16:03     ` Pedro Alves
  2015-01-06 15:58   ` Eli Zaretskii
  2015-01-16 16:34   ` Eli Zaretskii
  2 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2015-01-05 19:40 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Mon, 05 Jan 2015 19:11:21 +0000
> From: Pedro Alves <palves@redhat.com>
> 
> On 12/31/2014 05:45 PM, Eli Zaretskii wrote:
> > This patch fixes the problem whereby setting the border attributes had
> > no effect whatsoever.  
> 
> Rather, you need to switch out of the TUI and back, with c-x,a.

Yeah, well... not really friendly.

> > While at that, this also fixes a copy/paste error in a comment to a
> > function.
> 
> Could you please push this part separately?

Will do.

> > 2014-12-31  Eli Zaretskii  <eliz@gnu.org>
> > 
> > 	* tui/tui-win.c (tui_rehighlight_all): New function.
> > 
> 
> No empty lines between ChangeLog entries, please.

Why are we using a format that is different from how Emacs formats
ChangeLog entries?  It's just annoying extra manual work.

> > --- gdb/tui/tui-command.c~0	2014-10-29 21:45:50 +0200
> > +++ gdb/tui/tui-command.c	2014-12-31 13:49:43 +0200
> > @@ -50,7 +50,12 @@
> 
> Seems like you're not using git diff to generate the diffs
> for some reason.

This wasn't done in the git repo, I did it with the GDB 7.8.1 sources.

> If using GNU diff, could you make sure to use the "-p" switch?  It
> makes review a lot easier.

I'll try to remember.  (The ChangeLog entries state the function
names, don't they?)

> > --- gdb/tui/tui-command.c~0	2014-10-29 21:45:50 +0200
> > +++ gdb/tui/tui-command.c	2014-12-31 13:49:43 +0200
> > @@ -50,7 +50,12 @@
> >
> >    /* Handle the CTRL-L refresh for each window.  */
> >    if (ch == '\f')
> > -    tui_refresh_all_win ();
> > +    {
> > +      if (tui_update_variables ())
> > +	tui_rehighlight_all ();
> > +
> > +      tui_refresh_all_win ();
> 
> 
> tui_refresh_all_win already calls tui_check_and_display_highlight_if_needed,
> so why do we need to call tui_rehighlight_all?  Is it the tui_update_variables
> call that is really missing?

Yes, the call to tui_update_variables is required to take notice of
the changes.  Also, I'm not sure tui_refresh_all_win does that for all
windows, it has a switch that handles on 3 types.

> But why would we need to call tui_update_variables on CTRL-L, if we
> already call it when the user changes the variables?

I thought it'd be better to ensure this is called directly on refresh,
since C-l can be used in all kinds of weird situations where the
screen is messed up e.g. by the program being debugged.

> > --- gdb/tui/tui-hooks.c~0	2014-06-11 18:34:41 +0300
> > +++ gdb/tui/tui-hooks.c	2014-12-31 14:41:11 +0200
> > @@ -247,12 +247,23 @@
> >    tui_display_main ();
> >  }
> >
> > +/* Refresh the display when settings important to us change.  */
> > +static void
> > +tui_note_setting_change (const char *param, const char *value)
> > +{
> > +  if (tui_active
> > +      && strncmp (param, "tui ", sizeof ("tui ") - 1) == 0
> > +      && tui_update_variables ())
> > +    tui_rehighlight_all ();
> > +}
> > +
> 
> Please do this from the "set" hook of the relevant commands instead.
> IOW, replace NULL below (and in the other commands):
> 
>   add_setshow_enum_cmd ("active-border-mode", no_class, tui_border_mode_enums,
> 			&tui_active_border_mode, _("\
> ...
> 			NULL,
> 			show_tui_active_border_mode,
> 			&tui_setlist, &tui_showlist);

OK.

> > @@ -985,11 +995,14 @@
> >    /* Make sure the curses mode is enabled.  */
> >    tui_enable ();
> >
> > +  if (tui_update_variables ())
> > +    tui_rehighlight_all ();
> > +
> >    tui_refresh_all_win ();
> >  }
> >
> 
> I don't understand this one.  When is it ever necessary?

Same as Ctrl-l: this is the "refresh" command.

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

* Re: [PATCHSET] [2/4] Fix various issue in TUI
  2015-01-05 19:11 ` Pedro Alves
  2015-01-05 19:40   ` Eli Zaretskii
@ 2015-01-06 15:58   ` Eli Zaretskii
  2015-01-07 13:48     ` Pedro Alves
  2015-01-16 16:34   ` Eli Zaretskii
  2 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2015-01-06 15:58 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Mon, 05 Jan 2015 19:11:21 +0000
> From: Pedro Alves <palves@redhat.com>
> 
> > --- gdb/tui/tui-hooks.c~0	2014-06-11 18:34:41 +0300
> > +++ gdb/tui/tui-hooks.c	2014-12-31 14:41:11 +0200
> > @@ -247,12 +247,23 @@
> >    tui_display_main ();
> >  }
> >
> > +/* Refresh the display when settings important to us change.  */
> > +static void
> > +tui_note_setting_change (const char *param, const char *value)
> > +{
> > +  if (tui_active
> > +      && strncmp (param, "tui ", sizeof ("tui ") - 1) == 0
> > +      && tui_update_variables ())
> > +    tui_rehighlight_all ();
> > +}
> > +
> 
> Please do this from the "set" hook of the relevant commands instead.
> IOW, replace NULL below (and in the other commands):
> 
>   add_setshow_enum_cmd ("active-border-mode", no_class, tui_border_mode_enums,
> 			&tui_active_border_mode, _("\
> ...
> 			NULL,
> 			show_tui_active_border_mode,
> 			&tui_setlist, &tui_showlist);

Like this?

2015-01-06  Eli Zaretskii  <eliz@gnu.org>

	* tui/tui-win.c (tui_set_var_cmd): New function.
	(_initialize_tui_win) <border-kind, border-mode>:
	<active-border-mode>: Use tui_set_var_cmd as the "set" function.

--- gdb/tui/tui-win.c~2	2015-01-04 08:07:30 +0200
+++ gdb/tui/tui-win.c	2015-01-06 08:03:05 +0200
@@ -346,6 +346,12 @@ tui_get_cmd_list (void)
   return &tuilist;
 }
 
+void tui_set_var_cmd (char *null_args, int from_tty, struct cmd_list_element *c)
+{
+  if (tui_update_variables ())
+    tui_rehighlight_all ();
+}
+
 /* Function to initialize gdb commands, for tui window
    manipulation.  */
 
@@ -422,7 +428,7 @@ This variable controls the border of TUI
 space           use a white space\n\
 ascii           use ascii characters + - | for the border\n\
 acs             use the Alternate Character Set"),
-			NULL,
+			tui_set_var_cmd,
 			show_tui_border_kind,
 			&tui_setlist, &tui_showlist);
 
@@ -438,7 +444,7 @@ half            use half bright\n\
 half-standout   use half bright and standout mode\n\
 bold            use extra bright or bold\n\
 bold-standout   use extra bright or bold with standout mode"),
-			NULL,
+			tui_set_var_cmd,
 			show_tui_border_mode,
 			&tui_setlist, &tui_showlist);
 
@@ -454,7 +460,7 @@ half            use half bright\n\
 half-standout   use half bright and standout mode\n\
 bold            use extra bright or bold\n\
 bold-standout   use extra bright or bold with standout mode"),
-			NULL,
+			tui_set_var_cmd,
 			show_tui_active_border_mode,
 			&tui_setlist, &tui_showlist);
 }

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

* Re: [PATCHSET] [2/4] Fix various issue in TUI
  2015-01-05 19:40   ` Eli Zaretskii
@ 2015-01-06 16:03     ` Pedro Alves
  0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2015-01-06 16:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 01/05/2015 07:40 PM, Eli Zaretskii wrote:
>> Date: Mon, 05 Jan 2015 19:11:21 +0000
>> From: Pedro Alves <palves@redhat.com>
>>
>> On 12/31/2014 05:45 PM, Eli Zaretskii wrote:
>>> This patch fixes the problem whereby setting the border attributes had
>>> no effect whatsoever.  
>>
>> Rather, you need to switch out of the TUI and back, with c-x,a.
> 
> Yeah, well... not really friendly.

Yeah, definitely agreed.

>>> 2014-12-31  Eli Zaretskii  <eliz@gnu.org>
>>>
>>> 	* tui/tui-win.c (tui_rehighlight_all): New function.
>>>
>>
>> No empty lines between ChangeLog entries, please.
> 
> Why are we using a format that is different from how Emacs formats
> ChangeLog entries?  It's just annoying extra manual work.

That's a bit exaggerated, given you always have to edit
the ChangeLog entry anyway.  Both the emacs, and the gnu coding
conventions manuals say that when changes are related, they get no line
between:

  http://www.gnu.org/software/emacs/manual/html_node/emacs/Format-of-ChangeLog.html

"One entry can describe several changes; each change should have
its own item, or its own line in an item. Normally there should be a blank
line between items. When items are related (parts of the same change,
in different places), group them by leaving no blank line between them."

 https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs

"Separate unrelated change log entries with blank lines. DonÂ’t put blank lines
between individual changes of an entry. You can omit the file name and the asterisk
when successive individual changes are in the same file."

Given we prefer that unrelated changes are split into separate patches,
it follows that the norm is to not include the empty line.
emacs can't guess whether the changes are related or not, but IMO a much
better default would be to assume yes.

> 
>>> --- gdb/tui/tui-command.c~0	2014-10-29 21:45:50 +0200
>>> +++ gdb/tui/tui-command.c	2014-12-31 13:49:43 +0200
>>> @@ -50,7 +50,12 @@
>>
>> Seems like you're not using git diff to generate the diffs
>> for some reason.
> 
> This wasn't done in the git repo, I did it with the GDB 7.8.1 sources.
> 
>> If using GNU diff, could you make sure to use the "-p" switch?  It
>> makes review a lot easier.
> 
> I'll try to remember.

Thanks.

> (The ChangeLog entries state the function
> names, don't they?)

They do, but then without -p it's harder to map a change to the
corresponding ChangeLog entry, exactly because the hunk is missing
the function name that -p gives you.

> 
>>> --- gdb/tui/tui-command.c~0	2014-10-29 21:45:50 +0200
>>> +++ gdb/tui/tui-command.c	2014-12-31 13:49:43 +0200
>>> @@ -50,7 +50,12 @@
>>>
>>>    /* Handle the CTRL-L refresh for each window.  */
>>>    if (ch == '\f')
>>> -    tui_refresh_all_win ();
>>> +    {
>>> +      if (tui_update_variables ())
>>> +	tui_rehighlight_all ();
>>> +
>>> +      tui_refresh_all_win ();
>>
>>
>> tui_refresh_all_win already calls tui_check_and_display_highlight_if_needed,
>> so why do we need to call tui_rehighlight_all?  Is it the tui_update_variables
>> call that is really missing?
> 
> Yes, the call to tui_update_variables is required to take notice of
> the changes.  Also, I'm not sure tui_refresh_all_win does that for all
> windows, it has a switch that handles on 3 types.

If there's a window type that is mishandled in tui_refresh_all_win,
then we should fix it there.  I'd prefer to start out with the minimal
that that just fixes what we know is broken.

> 
>> But why would we need to call tui_update_variables on CTRL-L, if we
>> already call it when the user changes the variables?
> 
> I thought it'd be better to ensure this is called directly on refresh,
> since C-l can be used in all kinds of weird situations where the
> screen is messed up e.g. by the program being debugged.

If the screen is messed up, then we should always unconditionally
redraw everything, not do it only when the user has changed tui
settings, which is what tui_update_variables concerns about.  But
that's what tui_refresh_all_win does already, so I don't think that
change really does anything.  Unless there's a real reason, I'd rather
drop those hunks.

> 
>>> --- gdb/tui/tui-hooks.c~0	2014-06-11 18:34:41 +0300
>>> +++ gdb/tui/tui-hooks.c	2014-12-31 14:41:11 +0200
>>> @@ -247,12 +247,23 @@
>>>    tui_display_main ();
>>>  }
>>>
>>> +/* Refresh the display when settings important to us change.  */
>>> +static void
>>> +tui_note_setting_change (const char *param, const char *value)
>>> +{
>>> +  if (tui_active
>>> +      && strncmp (param, "tui ", sizeof ("tui ") - 1) == 0
>>> +      && tui_update_variables ())
>>> +    tui_rehighlight_all ();
>>> +}
>>> +
>>
>> Please do this from the "set" hook of the relevant commands instead.
>> IOW, replace NULL below (and in the other commands):
>>
>>   add_setshow_enum_cmd ("active-border-mode", no_class, tui_border_mode_enums,
>> 			&tui_active_border_mode, _("\
>> ...
>> 			NULL,
>> 			show_tui_active_border_mode,
>> 			&tui_setlist, &tui_showlist);
> 
> OK.
> 
>>> @@ -985,11 +995,14 @@
>>>    /* Make sure the curses mode is enabled.  */
>>>    tui_enable ();
>>>
>>> +  if (tui_update_variables ())
>>> +    tui_rehighlight_all ();
>>> +
>>>    tui_refresh_all_win ();
>>>  }
>>>
>>
>> I don't understand this one.  When is it ever necessary?
> 
> Same as Ctrl-l: this is the "refresh" command.

Then I don't think this is necessary.

It may also be simpler to just call tui_refresh_all_win
from the settings' set hook(s) instead of adding
the new tui_rehighlight_all function.  I don't imagine
a full redraw being a performance issue here?

Thanks,
Pedro Alves

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

* Re: [PATCHSET] [2/4] Fix various issue in TUI
  2015-01-06 15:58   ` Eli Zaretskii
@ 2015-01-07 13:48     ` Pedro Alves
  2015-01-16 16:29       ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2015-01-07 13:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 01/06/2015 03:55 PM, Eli Zaretskii wrote:

> 2015-01-06  Eli Zaretskii  <eliz@gnu.org>
> 
> 	* tui/tui-win.c (tui_set_var_cmd): New function.
> 	(_initialize_tui_win) <border-kind, border-mode>:
> 	<active-border-mode>: Use tui_set_var_cmd as the "set" function.
> 
> --- gdb/tui/tui-win.c~2	2015-01-04 08:07:30 +0200
> +++ gdb/tui/tui-win.c	2015-01-06 08:03:05 +0200
> @@ -346,6 +346,12 @@ tui_get_cmd_list (void)
>    return &tuilist;
>  }
>  
> +void tui_set_var_cmd (char *null_args, int from_tty, struct cmd_list_element *c)

Line break after "void".  Misses an intro comment, maybe something
like:

/* The set_func hook of "set tui ..." settings that affect
   the TUI display.  */

> +{
> +  if (tui_update_variables ())
> +    tui_rehighlight_all ();

The tui_rehighlight_all function is missing in this patch.
Did you indend for this to be tui_refresh_all_win?

Is this OK to call when the tui is not enabled?

Thanks,
Pedro Alves

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

* Re: [PATCHSET] [2/4] Fix various issue in TUI
  2015-01-07 13:48     ` Pedro Alves
@ 2015-01-16 16:29       ` Eli Zaretskii
  2015-01-16 16:59         ` Sergio Durigan Junior
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2015-01-16 16:29 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Wed, 07 Jan 2015 13:48:28 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> > +void tui_set_var_cmd (char *null_args, int from_tty, struct cmd_list_element *c)
> 
> Line break after "void".  Misses an intro comment, maybe something
> like:
> 
> /* The set_func hook of "set tui ..." settings that affect
>    the TUI display.  */

Fixed.

> > +{
> > +  if (tui_update_variables ())
> > +    tui_rehighlight_all ();
> 
> The tui_rehighlight_all function is missing in this patch.
> Did you indend for this to be tui_refresh_all_win?

No, I meant tui_rehighlight_all from the original patch.  See below.

> Is this OK to call when the tui is not enabled?

I added tui_active to the condition.

Here's what I pushed:

commit 6cdb25f4df143e8d98bd71bf943bbe61c702e239
Author: Eli Zaretskii <eliz@gnu.org>
Date:   Fri Jan 16 18:24:16 2015 +0200

    Make setting TUI border attributes take effect immediately
    
    gdb/
    2015-01-16  Eli Zaretskii  <eliz@gnu.org>
    
         * tui/tui-win.c (tui_rehighlight_all, tui_set_var_cmd): New
         functions.
         (_initialize_tui_win) <border-kind, border-mode>:
         <active-border-mode>: Use tui_set_var_cmd as the "set" function.
         * tui/tui-win.h: Add prototype for tui_rehighlight_all.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 661f9b6..e45f5c1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2015-01-16  Eli Zaretskii  <eliz@gnu.org>
 
+	* tui/tui-win.c (tui_rehighlight_all, tui_set_var_cmd): New
+	functions.
+	(_initialize_tui_win) <border-kind, border-mode>:
+	<active-border-mode>: Use tui_set_var_cmd as the "set" function.
+	* tui/tui-win.h: Add prototype for tui_rehighlight_all.
+
 	* tui/tui-win.c (tui_scroll_left_command, tui_scroll_right_command):
 	Doc fix.
 	(tui_set_tab_width_command): Delete and recreate the source and
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 96fa801..7e9bd1e 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -344,6 +344,15 @@ struct cmd_list_element **
   return &tuilist;
 }
 
+/* The set_func hook of "set tui ..." commands that affect the window
+   borders on the TUI display.  */
+void
+tui_set_var_cmd (char *null_args, int from_tty, struct cmd_list_element *c)
+{
+  if (tui_update_variables () && tui_active)
+    tui_rehighlight_all ();
+}
+
 /* Function to initialize gdb commands, for tui window
    manipulation.  */
 
@@ -420,7 +429,7 @@ Set the width (in characters) of tab stops.\n\
 space           use a white space\n\
 ascii           use ascii characters + - | for the border\n\
 acs             use the Alternate Character Set"),
-			NULL,
+			tui_set_var_cmd,
 			show_tui_border_kind,
 			&tui_setlist, &tui_showlist);
 
@@ -436,7 +445,7 @@ Set the width (in characters) of tab stops.\n\
 half-standout   use half bright and standout mode\n\
 bold            use extra bright or bold\n\
 bold-standout   use extra bright or bold with standout mode"),
-			NULL,
+			tui_set_var_cmd,
 			show_tui_border_mode,
 			&tui_setlist, &tui_showlist);
 
@@ -452,7 +461,7 @@ Set the width (in characters) of tab stops.\n\
 half-standout   use half bright and standout mode\n\
 bold            use extra bright or bold\n\
 bold-standout   use extra bright or bold with standout mode"),
-			NULL,
+			tui_set_var_cmd,
 			show_tui_active_border_mode,
 			&tui_setlist, &tui_showlist);
 }
@@ -646,6 +655,14 @@ Set the width (in characters) of tab stops.\n\
   tui_show_locator_content ();
 }
 
+void
+tui_rehighlight_all (void)
+{
+  enum tui_win_type type;
+
+  for (type = SRC_WIN; type < MAX_MAJOR_WINDOWS; type++)
+    tui_check_and_display_highlight_if_needed (tui_win_list[type]);
+}
 
 /* Resize all the windows based on the terminal size.  This function
    gets called from within the readline sinwinch handler.  */
diff --git a/gdb/tui/tui-win.h b/gdb/tui/tui-win.h
index 6601d4b..7d77a00 100644
--- a/gdb/tui/tui-win.h
+++ b/gdb/tui/tui-win.h
@@ -55,4 +55,7 @@ extern void tui_scroll (enum tui_scroll_direction,
 /* Create or get the TUI command list.  */
 struct cmd_list_element **tui_get_cmd_list (void);
 
+/* Set a TUI variable.  */
+void tui_set_var_cmd (char *, int, struct cmd_list_element *);
+
 #endif

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

* Re: [PATCHSET] [2/4] Fix various issue in TUI
  2015-01-05 19:11 ` Pedro Alves
  2015-01-05 19:40   ` Eli Zaretskii
  2015-01-06 15:58   ` Eli Zaretskii
@ 2015-01-16 16:34   ` Eli Zaretskii
  2 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2015-01-16 16:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Mon, 05 Jan 2015 19:11:21 +0000
> From: Pedro Alves <palves@redhat.com>
> 
> > While at that, this also fixes a copy/paste error in a comment to a
> > function.
> 
> Could you please push this part separately?

Done.

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

* Re: [PATCHSET] [2/4] Fix various issue in TUI
  2015-01-16 16:29       ` Eli Zaretskii
@ 2015-01-16 16:59         ` Sergio Durigan Junior
  2015-01-16 17:59           ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Sergio Durigan Junior @ 2015-01-16 16:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Pedro Alves, gdb-patches

On Friday, January 16 2015, Eli Zaretskii wrote:

> Here's what I pushed:
>
> commit 6cdb25f4df143e8d98bd71bf943bbe61c702e239
> Author: Eli Zaretskii <eliz@gnu.org>
> Date:   Fri Jan 16 18:24:16 2015 +0200
>
>     Make setting TUI border attributes take effect immediately
>     
>     gdb/
>     2015-01-16  Eli Zaretskii  <eliz@gnu.org>
>     
>          * tui/tui-win.c (tui_rehighlight_all, tui_set_var_cmd): New
>          functions.
>          (_initialize_tui_win) <border-kind, border-mode>:
>          <active-border-mode>: Use tui_set_var_cmd as the "set" function.
>          * tui/tui-win.h: Add prototype for tui_rehighlight_all.

Hi Eli,

This patch does not compile on x86_64 with --enable-targets=all.  You
can see the failures here:

  <http://gdb-build.sergiodj.net/builders/Fedora-x86_64-native-extended-gdbserver-m32/builds/14/steps/compile/logs/stdio>

> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 661f9b6..e45f5c1 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,11 @@
>  2015-01-16  Eli Zaretskii  <eliz@gnu.org>
>  
> +	* tui/tui-win.c (tui_rehighlight_all, tui_set_var_cmd): New
> +	functions.
> +	(_initialize_tui_win) <border-kind, border-mode>:
> +	<active-border-mode>: Use tui_set_var_cmd as the "set" function.
> +	* tui/tui-win.h: Add prototype for tui_rehighlight_all.
> +
>  	* tui/tui-win.c (tui_scroll_left_command, tui_scroll_right_command):
>  	Doc fix.
>  	(tui_set_tab_width_command): Delete and recreate the source and
> diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
> index 96fa801..7e9bd1e 100644
> --- a/gdb/tui/tui-win.c
> +++ b/gdb/tui/tui-win.c
> @@ -344,6 +344,15 @@ struct cmd_list_element **
>    return &tuilist;
>  }
>  
> +/* The set_func hook of "set tui ..." commands that affect the window
> +   borders on the TUI display.  */
> +void
> +tui_set_var_cmd (char *null_args, int from_tty, struct cmd_list_element *c)
> +{
> +  if (tui_update_variables () && tui_active)
> +    tui_rehighlight_all ();
> +}
> +
>  /* Function to initialize gdb commands, for tui window
>     manipulation.  */
>  
> @@ -420,7 +429,7 @@ Set the width (in characters) of tab stops.\n\
>  space           use a white space\n\
>  ascii           use ascii characters + - | for the border\n\
>  acs             use the Alternate Character Set"),
> -			NULL,
> +			tui_set_var_cmd,
>  			show_tui_border_kind,
>  			&tui_setlist, &tui_showlist);
>  
> @@ -436,7 +445,7 @@ Set the width (in characters) of tab stops.\n\
>  half-standout   use half bright and standout mode\n\
>  bold            use extra bright or bold\n\
>  bold-standout   use extra bright or bold with standout mode"),
> -			NULL,
> +			tui_set_var_cmd,
>  			show_tui_border_mode,
>  			&tui_setlist, &tui_showlist);
>  
> @@ -452,7 +461,7 @@ Set the width (in characters) of tab stops.\n\
>  half-standout   use half bright and standout mode\n\
>  bold            use extra bright or bold\n\
>  bold-standout   use extra bright or bold with standout mode"),
> -			NULL,
> +			tui_set_var_cmd,
>  			show_tui_active_border_mode,
>  			&tui_setlist, &tui_showlist);
>  }
> @@ -646,6 +655,14 @@ Set the width (in characters) of tab stops.\n\
>    tui_show_locator_content ();
>  }
>  
> +void
> +tui_rehighlight_all (void)
> +{
> +  enum tui_win_type type;
> +
> +  for (type = SRC_WIN; type < MAX_MAJOR_WINDOWS; type++)
> +    tui_check_and_display_highlight_if_needed (tui_win_list[type]);
> +}
>  
>  /* Resize all the windows based on the terminal size.  This function
>     gets called from within the readline sinwinch handler.  */
> diff --git a/gdb/tui/tui-win.h b/gdb/tui/tui-win.h
> index 6601d4b..7d77a00 100644
> --- a/gdb/tui/tui-win.h
> +++ b/gdb/tui/tui-win.h
> @@ -55,4 +55,7 @@ extern void tui_scroll (enum tui_scroll_direction,
>  /* Create or get the TUI command list.  */
>  struct cmd_list_element **tui_get_cmd_list (void);
>  
> +/* Set a TUI variable.  */
> +void tui_set_var_cmd (char *, int, struct cmd_list_element *);
> +
>  #endif

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

* Re: [PATCHSET] [2/4] Fix various issue in TUI
  2015-01-16 16:59         ` Sergio Durigan Junior
@ 2015-01-16 17:59           ` Eli Zaretskii
  0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2015-01-16 17:59 UTC (permalink / raw)
  To: Sergio Durigan Junior; +Cc: palves, gdb-patches

> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Cc: Pedro Alves <palves@redhat.com>, gdb-patches@sourceware.org
> Date: Fri, 16 Jan 2015 11:59:05 -0500
> 
> This patch does not compile on x86_64 with --enable-targets=all.  You
> can see the failures here:
> 
>   <http://gdb-build.sergiodj.net/builders/Fedora-x86_64-native-extended-gdbserver-m32/builds/14/steps/compile/logs/stdio>

Sorry, should be fixed now.

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

end of thread, other threads:[~2015-01-16 17:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-31 17:45 [PATCHSET] [2/4] Fix various issue in TUI Eli Zaretskii
2015-01-05 19:11 ` Pedro Alves
2015-01-05 19:40   ` Eli Zaretskii
2015-01-06 16:03     ` Pedro Alves
2015-01-06 15:58   ` Eli Zaretskii
2015-01-07 13:48     ` Pedro Alves
2015-01-16 16:29       ` Eli Zaretskii
2015-01-16 16:59         ` Sergio Durigan Junior
2015-01-16 17:59           ` Eli Zaretskii
2015-01-16 16:34   ` Eli Zaretskii

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).