Patchwork "with" command (alternative to the "/" command)

login
register
mail settings
Submitter Pedro Alves
Date May 24, 2019, 6:31 p.m.
Message ID <5220f37a-2bb8-d470-448d-5a640c940ef8@redhat.com>
Download mbox | patch
Permalink /patch/32852/
State New
Headers show

Comments

Pedro Alves - May 24, 2019, 6:31 p.m.
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 = <error reading variable: object size is larger than varsize-limit
> (gdb) /v
> $3 = "1234567890|1234567890|1234567890"
> (gdb) 
> 
> This might also be implemented with this patch e.g.
> (gdb) p larger
> $2 = <error reading variable: object size is larger than varsize-limit
> (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 <palves@redhat.com>
Date: Fri, 24 May 2019 18:55:46 +0100
Subject: [PATCH] with_command

---
 gdb/printcmd.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)
Philippe Waroquiers - May 25, 2019, 9:07 p.m.
On Fri, 2019-05-24 at 19:31 +0100, Pedro Alves wrote:
> 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.  :-(
No problem, the work done on / was not that huge, and at the end,
if it was the trigger that led to finish the option work, it was time
well spent :).

> > 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.
Looks nice and good enough to replace the / command
(and a very often 'with print some_option_often_temporary_changed -- COMMAND'
can always be put in a user defined command with a short name).

Thanks for doing this,

Philippe

Patch

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\