From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24589 invoked by alias); 5 Jan 2015 19:40:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 24556 invoked by uid 89); 5 Jan 2015 19:40:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_SOFTFAIL autolearn=no version=3.3.2 X-HELO: mtaout20.012.net.il Received: from mtaout20.012.net.il (HELO mtaout20.012.net.il) (80.179.55.166) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 05 Jan 2015 19:40:13 +0000 Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0NHP00A00YITA300@a-mtaout20.012.net.il> for gdb-patches@sourceware.org; Mon, 05 Jan 2015 21:40:11 +0200 (IST) Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0NHP00AQRYMY5930@a-mtaout20.012.net.il>; Mon, 05 Jan 2015 21:40:11 +0200 (IST) Date: Mon, 05 Jan 2015 19:40:00 -0000 From: Eli Zaretskii Subject: Re: [PATCHSET] [2/4] Fix various issue in TUI In-reply-to: <54AAE1D9.9000409@redhat.com> To: Pedro Alves Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83h9w5819b.fsf@gnu.org> References: <83y4pnbtnc.fsf@gnu.org> <54AAE1D9.9000409@redhat.com> X-IsSubscribed: yes X-SW-Source: 2015-01/txt/msg00056.txt.bz2 > Date: Mon, 05 Jan 2015 19:11:21 +0000 > From: Pedro Alves > > 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 > > > > * 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.