From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13826 invoked by alias); 10 Dec 2014 12:22:42 -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 13816 invoked by uid 89); 10 Dec 2014 12:22:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 10 Dec 2014 12:22:40 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sBACMZW0017387 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 10 Dec 2014 07:22:35 -0500 Received: from blade.nx (ovpn-116-21.ams2.redhat.com [10.36.116.21]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sBACMYfB013869; Wed, 10 Dec 2014 07:22:34 -0500 Received: by blade.nx (Postfix, from userid 1000) id C6DCD262981; Wed, 10 Dec 2014 12:22:33 +0000 (GMT) Date: Wed, 10 Dec 2014 12:22:00 -0000 From: Gary Benson To: Doug Evans Cc: gdb-patches@sourceware.org, Eli Zaretskii Subject: Re: [PATCH 3/3 v2] Implement completion limiting Message-ID: <20141210122233.GA7299@blade.nx> References: <1417094168-25868-1-git-send-email-gbenson@redhat.com> <1417094168-25868-4-git-send-email-gbenson@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-IsSubscribed: yes X-SW-Source: 2014-12/txt/msg00206.txt.bz2 Doug Evans wrote: > Gary Benson writes: > > This commit adds a new exception, TOO_MANY_COMPLETIONS_ERROR, to > > be thrown whenever the completer has generated too many candidates > > to be useful. A new user-settable variable, "max_completions", is > > added to control this behaviour. A top-level completion limit is > > added to complete_line_internal, as the final check to ensure the > > user never sees too many completions. An additional limit is > > added to default_make_symbol_completion_list_break_on, to halt > > time-consuming symbol table expansions. > > 1) IWBN if, when "Too many possibilities" is hit, the user was still > shown the completions thus far. I'd rather not have to abort the > command I'm trying to do, increase max-completions, and then try > again (or anything else to try to find what I'm looking for in order > to complete the command). At least not if I don't have to: the > completions thus far may provide a hint at what I'm looking for. > Plus GDB has already computed them, might as well print them. > Imagine if the total count is MAX+1, the user might find it annoying > to not be shown anything just because the count is one beyond the > max. > So instead of "Too many possibilities", how about printing the > completions thus far and then include a message saying the list is > clipped due to max-completions being reached? [Maybe readline makes > this difficult, but I think it'd be really nice have. Thoughts?] It's a nice idea but I'm not volunteering to implement it :) I already spent too much time figuring out how to thread things through readline. > 2) Readline has a limiting facility already: rl_completion_query_items. > But it's only applied after all completions have been computed so it > doesn't help us. > It would be good for the docs to explain the difference. > E.g. with the default of 200 for max-completions, if I do > > (top-gdb) b dwarf2 > > I get > > Display all 159 possibilities? (y or n) > > As a user, I'm kinda wondering why I'm being asked this if, for > example, I've explicitly set max-completions to some value. > [I know normally users might not even be aware of max-completions, > but suppose for the sake of discussion that they've set it to some > value larger than rl_completion_query_items.] > Note: rl_completion_query_items can be set in ~/.inputrc with > the completion-query-items parameter. rl_completion_query_items is a different thing. >From http://web.mit.edu/gnu/doc/html/rlman_2.html: Up to this many items will be displayed in response to a possible- completions call. After that, we ask the user if she is sure she wants to see them all. The default value is 100. Basically it's the threshold readline uses to decide whether to say "Display all 159 possibilities? (y or n)" or just print all the candidates without asking. > 3) rl_completion_query_items uses a value of zero to mean unlimited, > whereas max_completions uses -1 (or "unlimited"). While it might be > nice to provide a way to disable completions completely (by setting > max-completions to zero), I'm trying to decide whether that benefit > is sufficient to justify the inconsistency with > rl_completion_query_items. Thoughts? GDB's "remotetimeout" and "trace-buffer-size" options use -1 to denote unlimited, as do Scheme things defined with PARAM_ZUINTEGER_UNLIMITED. I prefer "max-completions" being consistent with other GDB options over "max-completions" being consistent with readline options. It's not important to me that users can disable completion, I put the functionality there because existing GDB code uses -1 so I had to handle 0 somehow. > 4) Is there a use-case that involves wanting both > rl_completion_query_items and max_completions parameters? > Certainly we need to limit the number while we're building them, > but do we need both parameters? [IOW, could we fold them into one?] > I can imagine wanting to set max-completions to some large value > but still be given a prompt if the completion-query-items > threshold is reached, so I think we want both. > I'm just raising the possibility that maybe we don't want both > in case someone wants to comment. I think we want both. > At the least, it'd probably be good to mention how both interact in > in the docs. I can do this. > > diff --git a/gdb/completer.c b/gdb/completer.c > > index a0f3fa3..4a2302c 100644 > > --- a/gdb/completer.c > > +++ b/gdb/completer.c > > [...] > > @@ -894,7 +984,35 @@ line_completion_function (const char *text, int matche> > [...] > > + if (rl_completion_type != TAB) > > + { > > +#if defined(TUI) > > + if (tui_active) > > + { > > + tui_puts ("\n"); > > + tui_puts (ex.message); > > + tui_puts ("\n"); > > + } > > + else > > +#endif > > + { > > + rl_crlf (); > > + fputs (ex.message, rl_outstream); > > + rl_crlf (); > > + } > > + > > + rl_on_new_line (); > > + } > > Bubbling up TUI implementation details into GDB core gives me pause. > I'm left wondering if there are more problems, and this is just > fixing one of them. I see that TUI has special code for readline, > (grep for readline in tui-io.c) so at the least I'm wondering why > this is necessary. > And if it is, let's push it down into tui/ as much as possible > (with a comment explaining why the code exists :-)). I'm no TUI expert, so I can't comment on whether it's necessary or not. Assuming it is necessary, I don't know how I could remove this block without vectorizing the CLI/TUI interface... or is this done already? There are other places where "#ifdef TUI" and variants are used, at least four: gdb/cli/cli-cmds.c gdb/main.c gdb/printcmd.c gdb/utils.c Cheers, Gary -- http://gbenson.net/