From patchwork Mon Mar 18 20:24:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 31902 Received: (qmail 124605 invoked by alias); 18 Mar 2019 20:24:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 124420 invoked by uid 89); 18 Mar 2019 20:24:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=role, Printing, Still, alongside X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 18 Mar 2019 20:24:32 +0000 Received: by mail-wr1-f65.google.com with SMTP id p10so300094wrq.1 for ; Mon, 18 Mar 2019 13:24:32 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id f21sm308322wmb.37.2019.03.18.13.24.29 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 18 Mar 2019 13:24:29 -0700 (PDT) Subject: Re: [PATCH v2] Fix first time you type UP or DOWN in TUI's command window (Re: [RFC 8.3 0/3] Some style fixes) To: Eli Zaretskii References: <20190308210433.32683-1-tromey@adacore.com> <83pnr08tc8.fsf@gnu.org> <83zhq26fcw.fsf@gnu.org> <874l899nh3.fsf@tromey.com> <8336ns3uv4.fsf@gnu.org> <83tvg4ywq2.fsf@gnu.org> Cc: tromey@adacore.com, gdb-patches@sourceware.org From: Pedro Alves Message-ID: <04e92055-4b62-5691-0425-d3110a862b31@redhat.com> Date: Mon, 18 Mar 2019 20:24:28 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <83tvg4ywq2.fsf@gnu.org> On 03/15/2019 03:37 PM, Eli Zaretskii wrote: >> Cc: gdb-patches@sourceware.org >> From: Pedro Alves >> Date: Fri, 15 Mar 2019 14:15:23 +0000 >> >> Bingo. >> >> So I think this is the right fix: >> >> >From 781ed3b6d82a4fb54f7bfe59185f0e6e9efd6b59 Mon Sep 17 00:00:00 2001 >> From: Pedro Alves >> Date: Fri, 15 Mar 2019 13:05:26 +0000 >> Subject: [PATCH] Fix first time you type UP or DOWN in TUI's command window >> >> The first time you type UP or DOWN arrow in the command window, GDB >> should scroll the source window, but instead it displays the line >> number and the file name in the command window(?). >> >> What happens there is that the first time we call >> tui_ui_out::do_field_int, it doesn't initialize m_line, because >> m_start_of_line is -1, as set by the constructor; and then the >> following call to tui_ui_out::do_field_string falls back to >> cli_ui_out::do_field_string because m_line is zero. >> >> The problem is caused by a typo in the C++ification of tui_ui_out, >> commit 112e8700a6f, where m_line and m_start_of_line's initial values >> were swapped from what they used to be: > > Thanks, this sound right to me. I've merged the patch to master and the 8.3 branch. > Still, I'd welcome some comments in > the header which explain the semantics of non-positive values of these > members, and for m_start_of_line, also its role in general. Fixing > this bug could have been much easier if that information was > available to begin with. For sure. This code predates me by a long shot. I wouldn't be surprised if it was already in the original TUI dump from HP. Anyway, I've stared at this for a while, and I _think_ this captures the idea. WDYT? From 8e37e7076f6ddca767db35b66284268935bcc186 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 18 Mar 2019 19:10:42 +0000 Subject: [PATCH] Add comments describing tui_ui_out and its fields, cleanup a bit This commit add comments describing tui_ui_out and its fields, and cleans up the code a little bit. Also switch to using in-class initialization so that the initial values can be seen alongside the comments. I see no reason for initializing m_line as -1 instead of 0, since all the checks in the .c file are of the form "> 0". AFAICS there's no practical difference between -1 and 0. So it seems simpler to initialize it as 0. There's a bit of redundancy in tui_ui_out::do_field_string, which is fixed by this commit. gdb/ChangeLog: 2019-03-18 Pedro Alves * tui/tui-out.c (tui_ui_out::do_field_string): Simplify. (tui_ui_out::do_text): Add comments. Reset M_LINE to 0 instead of to -1. Fix TABs vs spaces. (tui_ui_out::tui_ui_out): Don't initialize fields here. * tui/tui-out.h (tui_ui_out) Add intro comments. : In-class initialize, and add describing comment. --- gdb/tui/tui-out.c | 27 +++++++++++++-------------- gdb/tui/tui-out.h | 21 +++++++++++++++++++-- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/gdb/tui/tui-out.c b/gdb/tui/tui-out.c index dd37736c4a..64f77077c8 100644 --- a/gdb/tui/tui-out.c +++ b/gdb/tui/tui-out.c @@ -57,17 +57,13 @@ tui_ui_out::do_field_string (int fldno, int width, ui_align align, if (suppress_output ()) return; + m_start_of_line++; + if (fldname && m_line > 0 && strcmp (fldname, "fullname") == 0) { - m_start_of_line++; - if (m_line > 0) - { - tui_show_source (string, m_line); - } + tui_show_source (string, m_line); return; } - - m_start_of_line++; cli_ui_out::do_field_string (fldno, width, align, fldname, string, style); } @@ -94,11 +90,16 @@ tui_ui_out::do_text (const char *string) m_start_of_line++; if (m_line > 0) { + /* Printing a source line, so suppress regular output -- the + line was shown on the TUI's source window by tui_show_source + above instead. */ if (strchr (string, '\n') != 0) - { - m_line = -1; - m_start_of_line = 0; - } + { + /* If we've reached the end of the line, so go back to + letting text output go to the console. */ + m_line = 0; + m_start_of_line = 0; + } return; } if (strchr (string, '\n')) @@ -108,9 +109,7 @@ tui_ui_out::do_text (const char *string) } tui_ui_out::tui_ui_out (ui_file *stream) -: cli_ui_out (stream, 0), - m_line (-1), - m_start_of_line (0) + : cli_ui_out (stream, 0) { } diff --git a/gdb/tui/tui-out.h b/gdb/tui/tui-out.h index 10311c9255..b0d8b8d898 100644 --- a/gdb/tui/tui-out.h +++ b/gdb/tui/tui-out.h @@ -20,6 +20,10 @@ #include "cli-out.h" +/* An ui_out class for the TUI. This is just like the CLI's ui_out, + except that it overrides output methods to detect when a source + line is being printed and show the source in the TUI's source + window instead of printing the line in the console window. */ class tui_ui_out : public cli_ui_out { public: @@ -39,8 +43,21 @@ protected: private: - int m_line; - int m_start_of_line; + /* These fields are used to make print_source_lines show the source + in the TUI's source window instead of in the console. + M_START_OF_LINE is incremented whenever something is output to + the ui_out. If an integer field named "line" is printed on the + ui_out, and nothing else has been printed yet (both + M_START_OF_LINE and M_LINE are still 0), we assume + print_source_lines is starting to print a source line, and thus + record the line number in M_LINE. Afterwards, when we see a + string field named "fullname" being output, we take the fullname + and the recorded line and show the source line in the TUI's + source window. tui_ui_out::do_text() suppresses text output + until it sees an endline being printed, at which point these + variables are reset back to 0. */ + int m_line = 0; + int m_start_of_line = 0; }; extern tui_ui_out *tui_out_new (struct ui_file *stream);