From patchwork Fri May 24 18:31:21 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 32852 Received: (qmail 9896 invoked by alias); 24 May 2019 18:31:27 -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 9886 invoked by uid 89); 24 May 2019 18:31:27 -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=sick, uncomfortable, surprisingly, rethrow X-HELO: mail-wm1-f41.google.com Received: from mail-wm1-f41.google.com (HELO mail-wm1-f41.google.com) (209.85.128.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 24 May 2019 18:31:25 +0000 Received: by mail-wm1-f41.google.com with SMTP id i3so10310421wml.4 for ; Fri, 24 May 2019 11:31:25 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:4eeb:42ff:feef:f164? ([2001:8a0:f913:f700:4eeb:42ff:feef:f164]) by smtp.gmail.com with ESMTPSA id h8sm6924731wmf.5.2019.05.24.11.31.22 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Fri, 24 May 2019 11:31:22 -0700 (PDT) Subject: "with" command (alternative to the "/" command) To: Philippe Waroquiers , Tom Tromey References: <20190421134440.21100-1-philippe.waroquiers@skynet.be> <20190421134440.21100-2-philippe.waroquiers@skynet.be> <877ebidxff.fsf@tromey.com> <1556232239.22002.15.camel@skynet.be> <87imuv34rv.fsf@tromey.com> <1556686410.1511.3.camel@skynet.be> <1556740463.1511.6.camel@skynet.be> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <5220f37a-2bb8-d470-448d-5a640c940ef8@redhat.com> Date: Fri, 24 May 2019 19:31:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <1556740463.1511.6.camel@skynet.be> On 5/1/19 8:54 PM, Philippe Waroquiers wrote: > There are some differences between what this patch provides > and the "/" command. I am listing them below, and discuss > if these differences are worth keeping "/" or not. > > "/" provides a quicker/faster to type way to set options, > but it is less 'discoverable' than the completion feature, > which probably all GDB users know already (and would like > to have for the options). Right. I think TAB-completion is a must-have feature. > For very often options that share an initial word, we might > define an alternate option e.g. > print [-A | -array] [-i | -array-indexes] > if we believe these options are so frequently used that > they must be fast to type. > > Also, the idea is that t"/" command will allow to redo > the previous command with additional options, eg. > (gdb) p larger > $2 = (gdb) /v > $3 = "1234567890|1234567890|1234567890" > (gdb)  > > This might also be implemented with this patch e.g. > (gdb) p larger > $2 = (gdb) -v > $3 = "1234567890|1234567890|1234567890" > (gdb) > where -v would be a shortcut for relaunching the previous command as: > (gdb) p -varsize-list unlimited Larger > > This patch implies to add new options to existing commands > (such as the options added to > print) to have a quick way > to change them for one command. > It looks however easy to do (in > particular as some of > the code is shared with the 'set option' code). > > "/" changes the global options, then launches the given command, > and then restore the global options. > The given command can e.g. be a user/python defined command > that itself launches a bunch of other commands, which should > be influenced by the global settings. > The user must then (like with GDB 8.3) type a bunch of > 'set this' > 'set that' > launch the user defined command > 'reset this' > 'reset that' > Or define a new user command that automates the above. > I think this is the most useful property of the command. I'd like to explore other user interfaces for this. I'm aware that you've done a ton of work on the / command, which makes it uncomfortable for me to suggest it... I wish it was discussed before that; if it was and I missed it, I'm truly sorry. :-( > Then if the user types C-c while the middle command runs, > the options will not be restored. > Possibly we could allow a command to be optionally given > after the existing 'set command' (which means: > change the option, runs the given command, restore the option). > That is in fact the equivalent of the "/" command, but > for just one option. > (compare with the shell: > export DISPLAY=xxxx > some_command > versus > DISPLAY=xxxx some_command). > Then most of the "/" is still available (e.g. in user > defined commands), but without the alternate "/" letters > to activate. > Alternatively, the 'try/catch' in the CLI or the 'block > of command' patch I started and never finished some > years ago. > > In summary: IMO, there is not a huge set of reasons to > have both the "/" and this patch, or at least there are > reasonable ways to do what "/" provides, maybe with > little additional features such as: > * the optional COMMAND after  > 'set some_option [COMMAND]' > * add a systematic way to relaunch the previous command > by starting a line with a '-' option. > Right, so today I'm kind of sick with fever so I decided to prototype something, instead of working on what I should be working on. :-P. So I tried quickly prototyping a "with" command, which is just like "set", but sets the setting, runs the command and then restores the setting. (gdb) help with Temporarily set SETTING, run COMMAND, and restore SETTING. Usage: with SETTING -- COMMAND SETTING is any setting settable with the "set" command. E.g.: with language pascal -- print obj with print elements unlimited -- print obj I integrated it with TAB-completion, which I think makes it OK to not have shorter aliases like in the / command. That's the part that I dislike about / -- that you're adding another "language" to GDB, something else that users need to learn. It actually works surprisingly nicely, even if verbose: (gdb) with print elements unlimited -- print 1 $1 = 1 (gdb) with non-stop on -- show non-stop Controlling the inferior in non-stop mode is on. (gdb) show non-stop Controlling the inferior in non-stop mode is off. (gdb) with language pascal -- print 1 $2 = 1 (gdb) with language pascal -- show language The current source language is "pascal". (gdb) show language The current source language is "auto; currently c". (gdb) with print elements 100 -- with print object off -- print 1 $3 = 1 You can shorten things a bit though, as long as unambiguous: So this: (gdb) with print elements 100 -- with print object off -- print 1 is the same as: (gdb) wit p el 100 -- wit p o 0 -- p 1 We could add a "w" alias for "with", as "w" is not taken. Then we'd have: (gdb) w p el 100 -- w p o 0 -- p 1 As mentioned, TAB completion works nicely: (gdb) with p[TAB] pagination print prompt python (gdb) with print [TAB] address max-depth static-members array max-symbolic-offset symbol array-indexes null-stop symbol-filename asm-demangle object symbol-loading demangle pascal_static-members thread-events elements pretty type entry-values raw union frame-arguments repeats vtbl inferior-events sevenbit-strings (gdb) with print [TAB] The patch below applies on top of the users/palves/cli-options branch, in order to make use of the complete_command function, the one that allows recursing the completion to the nested COMMAND: (gdb) with print elements unlimited -- thread apply all - -ascending -c -q -s (gdb) with print elements unlimited -- print -[TAB] -address -max-depth -repeats -vtbl -array -null-stop -static-members -array-indexes -object -symbol -elements -pretty -union > Now, of course, if hundreds of GDB users are suddenly wearing > a yellow jacket, and go in the street destroying everything > while shouting 'we want the "/" command', then we might have > to rediscuss :). :-) Notes: - I'm requiring "--" to separate the setting from the command. I think it might be possible to do without that, but, well, this is a prototype. :-) Here's the patch. It's smaller than one might think. :-) From 22805567c5ca8d008f2898083f2027e9d3f1ea43 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 24 May 2019 18:55:46 +0100 Subject: [PATCH] with_command --- gdb/printcmd.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/gdb/printcmd.c b/gdb/printcmd.c index 0509360581e..6098dc57b33 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -48,6 +48,8 @@ #include "cli/cli-option.h" #include "cli/cli-script.h" #include "cli/cli-style.h" +#include "cli/cli-decode.h" +#include "cli/cli-setshow.h" #include "common/format.h" #include "source.h" #include "common/byte-vector.h" @@ -1289,6 +1291,103 @@ set_command (const char *exp, int from_tty) evaluate_expression (expr.get ()); } +/* The "with" command. */ + +static void +with_command (const char *exp, int from_tty) +{ + const char *delim = strstr (exp, "--"); + + if (delim == exp) + error (_("Missing setting before '--' delimiter\n")); + + if (delim == nullptr + || !isspace (delim[-1]) + || !(isspace (delim[2]) || delim[2] == '\0')) + error (_("Missing '--' delimiter\n")); + + cmd_list_element *set_cmd = lookup_cmd (&exp, setlist, "with", 0, 1); + + if (set_cmd == nullptr) + error (_("Unknown setting %s\n"), exp); + + std::string value = std::string (exp, delim - exp); + const char *nested_cmd = skip_spaces (delim + 2); + + std::string org_value; + + switch (set_cmd->var_type) + { + case var_boolean: + case var_auto_boolean: + case var_integer: + case var_zinteger: + case var_zuinteger_unlimited: + org_value = std::to_string (*(int *) set_cmd->var); + break; + case var_uinteger: + case var_zuinteger: + org_value = std::to_string (*(unsigned int *) set_cmd->var); + break; + case var_string: + case var_string_noescape: + case var_filename: + case var_optional_filename: + case var_enum: + org_value = *(const char **) set_cmd->var; + break; + default: + gdb_assert_not_reached ("unhandled var_type"); + } + + /* Tweak the setting to the new temporary value. */ + do_set_command (value.c_str (), from_tty, set_cmd); + + try + { + /* Execute the nested command. */ + execute_command (nested_cmd, from_tty); + } + catch (const gdb_exception &ex) + { + /* Restore the setting and rethrow. */ + do_set_command (org_value.c_str (), from_tty, set_cmd); + throw; + } + + /* Restore the setting. */ + do_set_command (org_value.c_str (), from_tty, set_cmd); +} + +/* See valprint.h. */ + +void +with_command_completer (struct cmd_list_element *ignore, + completion_tracker &tracker, + const char *text, const char * /*word*/) +{ + tracker.set_use_custom_word_point (true); + + const char *delim = strstr (text, "--"); + + if (delim == text + || delim == nullptr + || !isspace (delim[-1]) + || !(isspace (delim[2]) || delim[2] == '\0')) + { + std::string new_text = std::string ("set ") + text; + tracker.advance_custom_word_point_by (-(int) strlen ("with")); + complete_command (tracker, new_text.c_str ()); + return; + } + + /* We're past the "--" delimiter. Complete on the sub command. */ + const char *nested_cmd = skip_spaces (delim + 2); + tracker.advance_custom_word_point_by (nested_cmd - text); + complete_command (tracker, nested_cmd); +} + + static void info_symbol_command (const char *arg, int from_tty) { @@ -2750,6 +2849,16 @@ Like \"print\" but don't put in value history and don't print newline.\n\ Usage: output EXP\n\ This is useful in user-defined commands.")); + c = add_com ("with", class_vars, with_command, _("\ +Temporarily set SETTING, run COMMAND, and restore SETTING.\n\ +Usage: with SETTING -- COMMAND\n\ +SETTING is any setting settable with the \"set\" command.\n\ +E.g.:\n\ + with language pascal -- print obj\n\ + with print elements unlimited -- print obj\n\ +\n")); + set_cmd_completer_handle_brkchars (c, with_command_completer); + add_prefix_cmd ("set", class_vars, set_command, _("\ Evaluate expression EXP and assign result to variable VAR\n\ Usage: set VAR = EXP\n\