| Message ID | 1420689048-23538-1-git-send-email-patrick@parcs.ath.cx |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 17827 invoked by alias); 8 Jan 2015 03:51:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 17802 invoked by uid 89); 8 Jan 2015 03:51:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: mail-qc0-f179.google.com Received: from mail-qc0-f179.google.com (HELO mail-qc0-f179.google.com) (209.85.216.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 08 Jan 2015 03:51:02 +0000 Received: by mail-qc0-f179.google.com with SMTP id c9so297304qcz.10 for <gdb-patches@sourceware.org>; Wed, 07 Jan 2015 19:51:00 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=laOGgkbB2qqijbTfo5mzx7w992OGa3qyofVD7H/Sbak=; b=CnIMtihgqKbILYTtnT/t8pBESrlPrDLbtz1XWeq5Yl7nwpNDHauLc8yKJdLFlZrCfn dHXdZK0P8PrfWZcRrO1KYAtj07yJmwARm1XJT4tMnYQkYmNeJoX75Li2eal7gOZu90WZ DWUlWEA3w3l+425nS5zYX0m0k9zRl6Y3IbZsmBuoVORkuJcofCs33HRdREJGTEiVNdwA BeoqrsZ0E43FZ+R2p1XvTR9sbtuf7hWqJ/gRe+14LZQcrpAuLOupy5+iSS6JiiaD0jJH DgCmYZ0HK2rMvhr7J/qawDrmRlSDOzUsNEGPVGNcRQPsbZZBliKlwUe63ALQzU+3uX++ ZvQw== X-Gm-Message-State: ALoCoQmHPzuMZ0g+Lff6SNtclVJKrlBqqy8DRUYWZsxb4klH8hqz7pqq/R3dSEURtaggr8M5d7M0 X-Received: by 10.224.43.202 with SMTP id x10mr11183532qae.16.1420689060027; Wed, 07 Jan 2015 19:51:00 -0800 (PST) Received: from localhost.localdomain (ool-4353ac94.dyn.optonline.net. [67.83.172.148]) by mx.google.com with ESMTPSA id 43sm3017226qgb.17.2015.01.07.19.50.58 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 07 Jan 2015 19:50:59 -0800 (PST) From: Patrick Palka <patrick@parcs.ath.cx> To: gdb-patches@sourceware.org Cc: Patrick Palka <patrick@parcs.ath.cx> Subject: [PATCH] TUI: rewrite tui_query_hook() Date: Wed, 7 Jan 2015 22:50:48 -0500 Message-Id: <1420689048-23538-1-git-send-email-patrick@parcs.ath.cx> |
Commit Message
Patrick Palka
Jan. 8, 2015, 3:50 a.m. UTC
This patch rewrites tui_query_hook() to print things via tui_puts() and to read in a line of input via wgetnstr(). The main motivation for this rewrite is to get the backspace key to work correctly during a quit prompt so that the user can revise their answer before pressing enter. The backspace key now works correctly because we now use getstr() instead of successive calls to getch(). gdb/ChangeLog: * tui/tui-hooks.c (tui_query_hook): Rewrite to use tui_puts and wgetnstr. --- gdb/tui/tui-hooks.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-)
Comments
On 01/08/2015 03:50 AM, Patrick Palka wrote: > This patch rewrites tui_query_hook() to print things via tui_puts() and > to read in a line of input via wgetnstr(). The main motivation for this > rewrite is to get the backspace key to work correctly during a quit > prompt so that the user can revise their answer before pressing enter. > The backspace key now works correctly because we now use getstr() > instead of successive calls to getch(). Pagination, done in prompt_for_continue, gets away without this hook, as it's written in terms of readline. If we did the same to defaulted_query, I think the default code would just work for the TUI too. Did you consider that? Thanks, Pedro Alves
On Thu, Jan 8, 2015 at 6:03 AM, Pedro Alves <palves@redhat.com> wrote: > On 01/08/2015 03:50 AM, Patrick Palka wrote: >> This patch rewrites tui_query_hook() to print things via tui_puts() and >> to read in a line of input via wgetnstr(). The main motivation for this >> rewrite is to get the backspace key to work correctly during a quit >> prompt so that the user can revise their answer before pressing enter. >> The backspace key now works correctly because we now use getstr() >> instead of successive calls to getch(). > > Pagination, done in prompt_for_continue, gets away without this hook, as > it's written in terms of readline. If we did the same to defaulted_query, > I think the default code would just work for the TUI too. Did you > consider that? This is complicated by the fact that the default query code inserts annotations before and after the query. If we use gdb_readline_wrapper to print the query and wait for input then the 2nd annotation will not be printed in a timely manner because gdb_readline_wrapper blocks until a response is given by the user. > > Thanks, > Pedro Alves >
> From: Patrick Palka <patrick@parcs.ath.cx> > Cc: Patrick Palka <patrick@parcs.ath.cx> > Date: Wed, 7 Jan 2015 22:50:48 -0500 > > @@ -80,30 +80,18 @@ tui_query_hook (const char *msg, va_list argp) > echo (); > while (1) > { > - wrap_here (""); /* Flush any buffered output. */ > - gdb_flush (gdb_stdout); > + char response[2], answer; > > - fputs_filtered (question, gdb_stdout); > - printf_filtered (_("(y or n) ")); > + tui_puts (question); > + tui_puts (_("(y or n) ")); > > - wrap_here (""); > - gdb_flush (gdb_stdout); > - > - answer = tui_getc (stdin); > - clearerr (stdin); /* in case of C-d */ > - if (answer == EOF) /* C-d */ > + if (wgetnstr (win, response, 1) == ERR) Given the latest discussions about buffering, don't you need to call wrefresh after the second tui_puts? Thanks.
On Thu, Jan 8, 2015 at 8:31 AM, Eli Zaretskii <eliz@gnu.org> wrote: >> From: Patrick Palka <patrick@parcs.ath.cx> >> Cc: Patrick Palka <patrick@parcs.ath.cx> >> Date: Wed, 7 Jan 2015 22:50:48 -0500 >> >> @@ -80,30 +80,18 @@ tui_query_hook (const char *msg, va_list argp) >> echo (); >> while (1) >> { >> - wrap_here (""); /* Flush any buffered output. */ >> - gdb_flush (gdb_stdout); >> + char response[2], answer; >> >> - fputs_filtered (question, gdb_stdout); >> - printf_filtered (_("(y or n) ")); >> + tui_puts (question); >> + tui_puts (_("(y or n) ")); >> >> - wrap_here (""); >> - gdb_flush (gdb_stdout); >> - >> - answer = tui_getc (stdin); >> - clearerr (stdin); /* in case of C-d */ >> - if (answer == EOF) /* C-d */ >> + if (wgetnstr (win, response, 1) == ERR) > > Given the latest discussions about buffering, don't you need to call > wrefresh after the second tui_puts? I have been using this patch for a few months locally and have not seen any buffering issues. The recently-mentioned issues are mostly Windows-related, aren't they? I can add a wrefresh() if that is what's needed to prevent potential buffering issues. > > Thanks.
On 01/08/2015 12:40 PM, Patrick Palka wrote: > If we use > gdb_readline_wrapper to print the query and wait for input then the > 2nd annotation will not be printed in a timely manner because > gdb_readline_wrapper blocks until a response is given by the user. Can't we just cat the annotations bits into the query string itself? IOW, make them part of the secondary prompt. Thanks, Pedro Alves
On 01/08/2015 01:42 PM, Patrick Palka wrote: > On Thu, Jan 8, 2015 at 8:31 AM, Eli Zaretskii <eliz@gnu.org> wrote: >>> From: Patrick Palka <patrick@parcs.ath.cx> >>> Cc: Patrick Palka <patrick@parcs.ath.cx> >>> Date: Wed, 7 Jan 2015 22:50:48 -0500 >>> >>> @@ -80,30 +80,18 @@ tui_query_hook (const char *msg, va_list argp) >>> echo (); >>> while (1) >>> { >>> - wrap_here (""); /* Flush any buffered output. */ >>> - gdb_flush (gdb_stdout); >>> + char response[2], answer; >>> >>> - fputs_filtered (question, gdb_stdout); >>> - printf_filtered (_("(y or n) ")); >>> + tui_puts (question); >>> + tui_puts (_("(y or n) ")); >>> >>> - wrap_here (""); >>> - gdb_flush (gdb_stdout); >>> - >>> - answer = tui_getc (stdin); >>> - clearerr (stdin); /* in case of C-d */ >>> - if (answer == EOF) /* C-d */ >>> + if (wgetnstr (win, response, 1) == ERR) >> >> Given the latest discussions about buffering, don't you need to call >> wrefresh after the second tui_puts? > > I have been using this patch for a few months locally and have not > seen any buffering issues. That's because presently tui_puts always flushes. You'll need it since the query string doesn't end in a new line. This is exactly like we have a gdb_flush in defaulted_query after printing the query question. > The recently-mentioned issues are mostly > Windows-related, aren't they? I can add a wrefresh() if that is > what's needed to prevent potential buffering issues. Let's leave that either for the buffering patch, or if that patch goes in before this one, add the flushing call here then. That patch (or patches..) adds a function to handle the flushing, rather than calling wrefresh, and you'd want to call that one. Thanks, Pedro Alves
On Thu, Jan 8, 2015 at 8:53 AM, Pedro Alves <palves@redhat.com> wrote: > On 01/08/2015 12:40 PM, Patrick Palka wrote: > >> If we use >> gdb_readline_wrapper to print the query and wait for input then the >> 2nd annotation will not be printed in a timely manner because >> gdb_readline_wrapper blocks until a response is given by the user. > > Can't we just cat the annotations bits into the query string > itself? IOW, make them part of the secondary prompt. I'm not sure because the annotations contain newlines, so (with annotations enabled) the prompt passed to readline would have newlines. I do not know if readline supports multi-line prompts. But IMO a consolidation of the custom TUI query hook and the default query hook is a quite separate endeavor. I think it should be left as future work. > > Thanks, > Pedro Alves >
On Thu, Jan 8, 2015 at 9:10 AM, Patrick Palka <patrick@parcs.ath.cx> wrote: > On Thu, Jan 8, 2015 at 8:53 AM, Pedro Alves <palves@redhat.com> wrote: >> On 01/08/2015 12:40 PM, Patrick Palka wrote: >> >>> If we use >>> gdb_readline_wrapper to print the query and wait for input then the >>> 2nd annotation will not be printed in a timely manner because >>> gdb_readline_wrapper blocks until a response is given by the user. >> >> Can't we just cat the annotations bits into the query string >> itself? IOW, make them part of the secondary prompt. > > I'm not sure because the annotations contain newlines, so (with > annotations enabled) the prompt passed to readline would have > newlines. I do not know if readline supports multi-line prompts. > > But IMO a consolidation of the custom TUI query hook and the default > query hook is a quite separate endeavor. I think it should be left as > future work. And in the meantime this patch takes advantage of the existence of a custom TUI query hook to at least make queries within TUI to behave nicely. > >> >> Thanks, >> Pedro Alves >>
> From: Patrick Palka <patrick@parcs.ath.cx> > Date: Thu, 8 Jan 2015 08:42:38 -0500 > Cc: gdb-patches@sourceware.org > > > Given the latest discussions about buffering, don't you need to call > > wrefresh after the second tui_puts? > > I have been using this patch for a few months locally and have not > seen any buffering issues. The recently-mentioned issues are mostly > Windows-related, aren't they? I can add a wrefresh() if that is > what's needed to prevent potential buffering issues. I think we shouldn't assume that stdout gets flushed when a program reads from stdin.
On Thu, Jan 8, 2015 at 9:10 AM, Patrick Palka <patrick@parcs.ath.cx> wrote: > On Thu, Jan 8, 2015 at 8:53 AM, Pedro Alves <palves@redhat.com> wrote: >> On 01/08/2015 12:40 PM, Patrick Palka wrote: >> >>> If we use >>> gdb_readline_wrapper to print the query and wait for input then the >>> 2nd annotation will not be printed in a timely manner because >>> gdb_readline_wrapper blocks until a response is given by the user. >> >> Can't we just cat the annotations bits into the query string >> itself? IOW, make them part of the secondary prompt. > > I'm not sure because the annotations contain newlines, so (with > annotations enabled) the prompt passed to readline would have > newlines. I do not know if readline supports multi-line prompts. Actually readline does support multi-line prompts because what you mentioned is exactly what prompt_for_continue() does. It concatenates the annotations to the prompt string and passes the multi-line prompt to gdb_readline_wrapper(). So it will be easy to consolidate these two query hooks. I will try to do so. > > But IMO a consolidation of the custom TUI query hook and the default > query hook is a quite separate endeavor. I think it should be left as > future work. > >> >> Thanks, >> Pedro Alves >>
On Thu, Jan 8, 2015 at 9:14 AM, Eli Zaretskii <eliz@gnu.org> wrote: >> From: Patrick Palka <patrick@parcs.ath.cx> >> Date: Thu, 8 Jan 2015 08:42:38 -0500 >> Cc: gdb-patches@sourceware.org >> >> > Given the latest discussions about buffering, don't you need to call >> > wrefresh after the second tui_puts? >> >> I have been using this patch for a few months locally and have not >> seen any buffering issues. The recently-mentioned issues are mostly >> Windows-related, aren't they? I can add a wrefresh() if that is >> what's needed to prevent potential buffering issues. > > I think we shouldn't assume that stdout gets flushed when a program > reads from stdin. That sounds like a good rule of thumb.
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c index 6ba6285..9dee840 100644 --- a/gdb/tui/tui-hooks.c +++ b/gdb/tui/tui-hooks.c @@ -68,9 +68,9 @@ tui_query_hook (const char *msg, va_list argp) { int retval; int ans2; - int answer; char *question; struct cleanup *old_chain; + WINDOW *win = TUI_CMD_WIN->generic.handle; /* Format the question outside of the loop, to avoid reusing ARGP. */ @@ -80,30 +80,18 @@ tui_query_hook (const char *msg, va_list argp) echo (); while (1) { - wrap_here (""); /* Flush any buffered output. */ - gdb_flush (gdb_stdout); + char response[2], answer; - fputs_filtered (question, gdb_stdout); - printf_filtered (_("(y or n) ")); + tui_puts (question); + tui_puts (_("(y or n) ")); - wrap_here (""); - gdb_flush (gdb_stdout); - - answer = tui_getc (stdin); - clearerr (stdin); /* in case of C-d */ - if (answer == EOF) /* C-d */ + if (wgetnstr (win, response, 1) == ERR) { retval = 1; break; } - /* Eat rest of input line, to EOF or newline. */ - if (answer != '\n') - do - { - ans2 = tui_getc (stdin); - clearerr (stdin); - } - while (ans2 != EOF && ans2 != '\n' && ans2 != '\r'); + + answer = response[0]; if (answer >= 'a') answer -= 040; @@ -117,10 +105,13 @@ tui_query_hook (const char *msg, va_list argp) retval = 0; break; } - printf_filtered (_("Please answer y or n.\n")); + tui_puts (_("Please answer y or n.\n")); } noecho (); + /* Update our knowledge of the cursor position. */ + tui_puts (""); + do_cleanups (old_chain); return retval; }