Quiet command regex instructions during batch execution

[kastiglione]

Summary

Within .lldbinit, regex commands can be structured as a list of substitutions over
multiple lines. It's possible that this is uninentional, but it works and has
benefits.

For example:

command regex <command-name>
s/pat1/repl1/
s/pat2/repl2/
...

I use this form of command regex in my ~/.lldbinit, because it makes it
clearer to write and read compared to a single line definition, because
multiline substitutions don't need to be quoted, and are broken up one per line.

However, multiline definitions result in usage instructions being printed for
each use. The result is that every time I run lldb, I get a dozen or more
lines of noise. With this change, the instructions are only printed when
command regex is invoked interactively, or from a terminal, neither of which
are true when lldb is sourcing ~/.lldbinit.

[jingham]

This behavior change seems desirable. But there are other commands that use the IOHandlerActivated method to print instructions like this, and should get the same treatment (breakpoint command add, type summary add...). It seems ugly to have to do this one by one in individual commands.

The IOHandlerDelegate is a pretty general class, so I don't think it is right for the IOHandler to only dispatch "IOHandlerActivated" to it when interactive. There's no requirement that the work you do in this method is only useful when interactive. But it might work if the IOHandlerDelegate could say whether it wants to be notified for IOHandlerActivated when non-interactive (like have a "ShouldNotifyWhenNonInteractive" method), and then the IOHandler could dispatch it or not based on the response to that method.

[kastiglione]

Thanks @jingham.

if the IOHandlerDelegate could say whether it wants to be notified for IOHandlerActivated when non-interactive (like have a "ShouldNotifyWhenNonInteractive" method), and then the IOHandler could dispatch it or not based on the response to that method

So the "ShouldNotifyWhenNonInteractive" would only control whether IOHandlerActivated is called, not any of the other IOHandlerDelegate methods?

What do you think about adding a IOHandlerActivatedInteractivelydelegate method? This could be called in addition to IOHandlerActivated, but only for interactive IO?

[clayborg]

source/Commands/CommandObjectCommands.cpp

979

Why do we need to check for a real terminal here? Isn't checking if it is interactive enough?

[jingham]

Sure, that also sounds fine.

Jim

[kastiglione]

Added IOHandlerActivatedInteractively

[kastiglione]

There are other commands that print instructions in the same way that command regex does, should they be updated in this change too?

[kastiglione]

@jingham If you're still willing to review this change to command regex, it is updated per our previous discussion, thanks.

[clayborg]

Seems like we should just add a "bool interactive" as a second parameter to "IOHandlerActivated". Then it will be easy to find the other places that need to be fixed up.

include/lldb/Core/IOHandler.h

203

Maybe remove the function below and add a "bool interactive" as a second parameter to this function?

source/Commands/CommandObjectCommands.cpp

978

See above inline comment about leaving the name the same but adding the "bool interactive" as a paramter

979–980

If we make changes I requested above this would become:

if (output_sp && interactive)

source/Core/IOHandler.cpp

334–335

m_delegate. IOHandlerActivated(*this, GetIsInteractive());

[kastiglione]

Add interactive parameter

[kastiglione]

@clayborg this has been updated the approach you suggested, adding an interactive parameter.

[kastiglione]

Thanks for reviewing @clayborg.

include/lldb/Core/IOHandler.h

 public:
   enum class Completion { None, LLDBCommand, Expression };
 
   IOHandlerDelegate(Completion completion = Completion::None)
       : m_completion(completion) {}
 
   virtual ~IOHandlerDelegate() = default;
 
-  virtual void IOHandlerActivated(IOHandler &io_handler) {}
+  virtual void IOHandlerActivated(IOHandler &io_handler, bool interactive) {}
 
   virtual void IOHandlerDeactivated(IOHandler &io_handler) {}
 
   virtual int IOHandlerComplete(IOHandler &io_handler, const char *current_line,
                                 const char *cursor, const char *last_char,
                                 int skip_first_n_matches, int max_matches,
                                 StringList &matches, StringList &descriptions);
 

include/lldb/Expression/REPL.h

public:
 
   lldb::IOHandlerSP GetIOHandler();
 
   Status RunLoop();
 
   //------------------------------------------------------------------
   // IOHandler::Delegate functions
   //------------------------------------------------------------------
-  void IOHandlerActivated(IOHandler &io_handler) override;
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override;
 
   bool IOHandlerInterrupt(IOHandler &io_handler) override;
 
   void IOHandlerInputInterrupted(IOHandler &io_handler,
                                  std::string &line) override;
 
   const char *IOHandlerGetFixIndentationCharacters() override;
 

lit/Commands/command-regex-delete.test

 # Check basic functionality of command regex.
 # RUN: %lldb -s %s 2>&1 | FileCheck %s
 
 command regex 'Help__'
-# CHECK: Enter one of more sed substitution commands in the form
+# CHECK: Enter one or more sed substitution commands in the form
 # We need to leave a new line after to end the regex.
 s/^$/help/
 
 Help__
 # CHECK: Debugger commands:
 
 command delete Help__
 Help__
 # CHECK: error: 'Help__' is not a valid command

lit/Commands/command-regex-unalias.test

 # Check that commands created with command regex cannot be unaliased
 # RUN: %lldb -s %s 2>&1 | FileCheck %s
 
 command regex 'Help__'
-# CHECK: Enter one of more sed substitution commands in the form
+# CHECK: Enter one or more sed substitution commands in the form
 # We need to leave a new line after to end the regex.
 s/^$/help/
 
 command unalias Help__
 Help__
 # CHECK: error: 'Help__' is not an alias

source/Commands/CommandObjectBreakpointCommand.cpp

are no syntax errors may indicate that a function was declared but never called.");
     // Push the data for the first argument into the m_arguments vector.
     m_arguments.push_back(arg);
   }
 
   ~CommandObjectBreakpointCommandAdd() override = default;
 
   Options *GetOptions() override { return &m_options; }
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
     StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-    if (output_sp) {
+    if (output_sp && interactive) {
       output_sp->PutCString(g_reader_instructions);
       output_sp->Flush();
     }
   }
 
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &line) override {
     io_handler.SetIsDone(true);

source/Commands/CommandObjectCommands.cpp

a number follows 'f':"
         R"(
 
     (lldb) command regex f s/^$/finish/ 's/([0-9]+)/frame select %1/')");
   }
 
   ~CommandObjectCommandsAddRegex() override = default;
 
 protected:
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
     StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-    if (output_sp) {
-      output_sp->PutCString("Enter one of more sed substitution commands in "
+    if (output_sp && interactive) {
+      output_sp->PutCString("Enter one or more sed substitution commands in "
                             "the form: 's/<regex>/<subst>/'.\nTerminate the "
                             "substitution list with an empty line.\n");
       output_sp->Flush();
     }
   }
 
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &data) override {
public:
     // Instance variables to hold the values for command options.
 
     std::string m_class_name;
     std::string m_funct_name;
     std::string m_short_help;
     ScriptedCommandSynchronicity m_synchronicity;
   };
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
     StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-    if (output_sp) {
+    if (output_sp && interactive) {
       output_sp->PutCString(g_python_command_instructions);
       output_sp->Flush();
     }
   }
 
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &data) override {
     StreamFileSP error_sp = io_handler.GetErrorStreamFile();

source/Commands/CommandObjectTarget.cpp

CommandObjectTargetStopHookAdd(CommandInterpreter &interpreter)
                                    IOHandlerDelegate::Completion::LLDBCommand),
         m_options() {}
 
   ~CommandObjectTargetStopHookAdd() override = default;
 
   Options *GetOptions() override { return &m_options; }
 
 protected:
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
     StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-    if (output_sp) {
+    if (output_sp && interactive) {
       output_sp->PutCString(
           "Enter your stop hook command(s).  Type 'DONE' to end.\n");
       output_sp->Flush();
     }
   }
 
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &line) override {

source/Commands/CommandObjectType.cpp

 
 public:
   enum SummaryFormatType { eRegularSummary, eRegexSummary, eNamedSummary };
 
   CommandObjectTypeSummaryAdd(CommandInterpreter &interpreter);
 
   ~CommandObjectTypeSummaryAdd() override = default;
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
     static const char *g_summary_addreader_instructions =
         "Enter your Python command(s). Type 'DONE' to end.\n"
         "def function (valobj,internal_dict):\n"
         "     \"\"\"valobj: an SBValue which you want to provide a summary "
         "for\n"
         "        internal_dict: an LLDB support object not to be used\"\"\"\n";
 
     StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-    if (output_sp) {
+    if (output_sp && interactive) {
       output_sp->PutCString(g_summary_addreader_instructions);
       output_sp->Flush();
     }
   }
 
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &data) override {
     StreamFileSP error_sp = io_handler.GetErrorStreamFile();
else {
       result.AppendError("must either provide a children list, a Python class "
                          "name, or use -P and type a Python class "
                          "line-by-line");
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
   }
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
     StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-    if (output_sp) {
+    if (output_sp && interactive) {
       output_sp->PutCString(g_synth_addreader_instructions);
       output_sp->Flush();
     }
   }
 
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &data) override {
     StreamFileSP error_sp = io_handler.GetErrorStreamFile();

source/Commands/CommandObjectWatchpointCommand.cpp

are no syntax errors may indicate that a function was declared but never called.");
     // Push the data for the first argument into the m_arguments vector.
     m_arguments.push_back(arg);
   }
 
   ~CommandObjectWatchpointCommandAdd() override = default;
 
   Options *GetOptions() override { return &m_options; }
 
-  void IOHandlerActivated(IOHandler &io_handler) override {
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override {
     StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-    if (output_sp) {
+    if (output_sp && interactive) {
       output_sp->PutCString(
           "Enter your debugger command(s).  Type 'DONE' to end.\n");
       output_sp->Flush();
     }
   }
 
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &line) override {

source/Core/IOHandler.cpp

 IOHandlerEditline::~IOHandlerEditline() {
 #ifndef LLDB_DISABLE_LIBEDIT
   m_editline_up.reset();
 #endif
 }
 
 void IOHandlerEditline::Activate() {
   IOHandler::Activate();
-  m_delegate.IOHandlerActivated(*this);
+  m_delegate.IOHandlerActivated(*this, GetIsInteractive());
 }
 
 void IOHandlerEditline::Deactivate() {
   IOHandler::Deactivate();
   m_delegate.IOHandlerDeactivated(*this);
 }
 
 bool IOHandlerEditline::GetLine(std::string &line, bool &interrupted) {
 #ifndef LLDB_DISABLE_LIBEDIT

source/Expression/REPL.cpp

if (!m_io_handler_sp) {
     } else {
       m_indent_str.clear();
       m_enable_auto_indent = false;
     }
   }
   return m_io_handler_sp;
 }
 
-void REPL::IOHandlerActivated(IOHandler &io_handler) {
+void REPL::IOHandlerActivated(IOHandler &io_handler, bool interactive) {
   lldb::ProcessSP process_sp = m_target.GetProcessSP();
   if (process_sp && process_sp->IsAlive())
     return;
   lldb::StreamFileSP error_sp(io_handler.GetErrorStreamFile());
   error_sp->Printf("REPL requires a running target process.\n");
   io_handler.SetIsDone(true);
 }
 

source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

#endif
   void SetThreadState(PyThreadState *s) {
     if (s)
       m_command_thread_state = s;
   }
 
   //----------------------------------------------------------------------
   // IOHandlerDelegate
   //----------------------------------------------------------------------
-  void IOHandlerActivated(IOHandler &io_handler) override;
+  void IOHandlerActivated(IOHandler &io_handler, bool interactive) override;
 
   void IOHandlerInputComplete(IOHandler &io_handler,
                               std::string &data) override;
 
   //------------------------------------------------------------------
   // Static Functions
   //------------------------------------------------------------------
   static void Initialize();

source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

 }
 
 lldb_private::ConstString ScriptInterpreterPython::GetPluginName() {
   return GetPluginNameStatic();
 }
 
 uint32_t ScriptInterpreterPython::GetPluginVersion() { return 1; }
 
-void ScriptInterpreterPython::IOHandlerActivated(IOHandler &io_handler) {
+void ScriptInterpreterPython::IOHandlerActivated(IOHandler &io_handler, bool interactive) {
   const char *instructions = nullptr;
 
   switch (m_active_io_handler) {
   case eIOHandlerNone:
     break;
   case eIOHandlerBreakpoint:
     instructions = R"(Enter your Python command(s). Type 'DONE' to end.
 def function (frame, bp_loc, internal_dict):
     """frame: the lldb.SBFrame for the location at which you stopped
        bp_loc: an lldb.SBBreakpointLocation for the breakpoint location information
        internal_dict: an LLDB support object not to be used"""
 )";
     break;
   case eIOHandlerWatchpoint:
     instructions = "Enter your Python command(s). Type 'DONE' to end.\n";
     break;
   }
 
   if (instructions) {
     StreamFileSP output_sp(io_handler.GetOutputStreamFile());
-    if (output_sp) {
+    if (output_sp && interactive) {
       output_sp->PutCString(instructions);
       output_sp->Flush();
     }
   }
 }
 
 void ScriptInterpreterPython::IOHandlerInputComplete(IOHandler &io_handler,
                                                      std::string &data) {