[lldb] Still echo the command if we print the error.

[JDevlieghere]

When the command interpreter is asked to not echo commands but still print errors, a user has no idea what command caused the error.

For example, when I add bogus in my ~/.lldbinit:

$ lldb
error: 'bogus' is not a valid command.

Things are even more confusing when we have inline diagnostics, which point to nothing. For example, when I add settings set target.run-args -foo to my ~/.lldbinit:

❯ lldb
                                    ˄˜˜˜
                                    ╰─ error: unknown or ambiguous option

We should still echo the command if the command fails, making it obvious which command caused the failure and fixing the inline diagnostics.

Fixes #171514

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

When the command interpreter is asked to not echo commands but still print errors, a user has no idea what command caused the error.

For example, when I add bogus in my ~/.lldbinit:

$ lldb
error: 'bogus' is not a valid command.

Things are even more confusing when we have inline diagnostics, which point to nothing. For example, when I add settings set target.run-args -foo to my ~/.lldbinit:

❯ lldb
                                    ˄˜˜˜
                                    ╰─ error: unknown or ambiguous option

We should still echo the command if the command fails, making it obvious which command caused the failure and fixing the inline diagnostics.

Fixes #171514

---

Full diff: https://github.com/llvm/llvm-project/pull/171931.diff

3 Files Affected:

• (modified) lldb/source/Interpreter/CommandInterpreter.cpp (+18-4)
• (added) lldb/test/Shell/Settings/Inputs/FailedCommand.in (+4)
• (added) lldb/test/Shell/Settings/TestEchoFailedCommands.test (+10)

diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index afc1753e21c46..1e359f9d08ed5 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3283,6 +3283,7 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
     if (line.empty())
       return;
   }
+  bool echoed_command = false;
   if (!is_interactive) {
     // When using a non-interactive file handle (like when sourcing commands
     // from a file) we need to echo the command out so we don't just see the
@@ -3291,6 +3292,7 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
       LockedStreamFile locked_stream =
           io_handler.GetOutputStreamFileSP()->Lock();
       locked_stream.Printf("%s%s\n", io_handler.GetPrompt(), line.c_str());
+      echoed_command = true;
     }
   }
 
@@ -3310,10 +3312,22 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
   lldb_private::CommandReturnObject result(m_debugger.GetUseColor());
   HandleCommand(line.c_str(), eLazyBoolCalculate, result);
 
-  // Now emit the command output text from the command we just executed
-  if ((result.Succeeded() &&
-       io_handler.GetFlags().Test(eHandleCommandFlagPrintResult)) ||
-      io_handler.GetFlags().Test(eHandleCommandFlagPrintErrors)) {
+  const bool print_result =
+      result.Succeeded() &&
+      io_handler.GetFlags().Test(eHandleCommandFlagPrintResult);
+  const bool print_error =
+      io_handler.GetFlags().Test(eHandleCommandFlagPrintErrors);
+
+  // Now emit the command output text from the command we just executed.
+  if (print_result || print_error) {
+    // If the command failed and we didn't echo it, echo it now so the user
+    // knows which command produced the error.
+    if (!echoed_command && !result.Succeeded() && print_error) {
+      LockedStreamFile locked_stream =
+          io_handler.GetOutputStreamFileSP()->Lock();
+      locked_stream.Printf("%s%s\n", io_handler.GetPrompt(), line.c_str());
+    }
+
     auto DefaultPrintCallback = [&](const CommandReturnObject &result) {
       // Display any inline diagnostics first.
       const bool inline_diagnostics = !result.GetImmediateErrorStream() &&
diff --git a/lldb/test/Shell/Settings/Inputs/FailedCommand.in b/lldb/test/Shell/Settings/Inputs/FailedCommand.in
new file mode 100644
index 0000000000000..c3bc5c704fe34
--- /dev/null
+++ b/lldb/test/Shell/Settings/Inputs/FailedCommand.in
@@ -0,0 +1,4 @@
+# This should succeed and not be echoed.
+expr 1+2
+# This should fail and be echoed.
+bogus_command
diff --git a/lldb/test/Shell/Settings/TestEchoFailedCommands.test b/lldb/test/Shell/Settings/TestEchoFailedCommands.test
new file mode 100644
index 0000000000000..3bb465707a41d
--- /dev/null
+++ b/lldb/test/Shell/Settings/TestEchoFailedCommands.test
@@ -0,0 +1,10 @@
+# Test that failed commands are echoed even when echoing is disabled.
+# This ensures users can see which command produced an error.
+
+RUN: mkdir -p %t.home
+RUN: cp %S/Inputs/FailedCommand.in %t.home/.lldbinit
+RUN: env HOME=%t.home %lldb-init -b 2>&1 | FileCheck %s
+
+CHECK-NOT: expr 1+2
+CHECK: (lldb) bogus_command
+CHECK: error: 'bogus_command' is not a valid command

[adrian-prantl]

not important, but
locked_stream << io_handler.GetPrompt() << line << "\n";
would avoid a bunch of length computation.