https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/111891
>From 4493bf07c8b18dac39a2a421f97fa34cd15a6031 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 10 Oct 2024 14:48:08 -0400 Subject: [PATCH 1/3] [LLDB]Provide clearer error message for invalid commands. Sometimes users (esp. gdb-longtime users) accidentally use GDB syntax, such as `breakpoint foo`, and they would get an error message from LLDB saying simply `Invalid command "breakpoint foo"`, which is not very helpful. This change provides additional suggestions to help correcting the mistake. --- .../lldb/Interpreter/CommandObjectMultiword.h | 2 + .../Commands/CommandObjectMultiword.cpp | 42 ++++++++++++++----- .../Commands/command-breakpoint-col.test | 2 + 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandObjectMultiword.h b/lldb/include/lldb/Interpreter/CommandObjectMultiword.h index bceb7f0e51edb6..cee118c3f454b5 100644 --- a/lldb/include/lldb/Interpreter/CommandObjectMultiword.h +++ b/lldb/include/lldb/Interpreter/CommandObjectMultiword.h @@ -70,6 +70,8 @@ class CommandObjectMultiword : public CommandObject { return m_subcommand_dict; } + std::string GetTopSubcommands(int count); + CommandObject::CommandMap m_subcommand_dict; bool m_can_be_removed; }; diff --git a/lldb/source/Commands/CommandObjectMultiword.cpp b/lldb/source/Commands/CommandObjectMultiword.cpp index 4efa5652a71703..f7bb58187895b4 100644 --- a/lldb/source/Commands/CommandObjectMultiword.cpp +++ b/lldb/source/Commands/CommandObjectMultiword.cpp @@ -194,28 +194,50 @@ void CommandObjectMultiword::Execute(const char *args_string, std::string error_msg; const size_t num_subcmd_matches = matches.GetSize(); - if (num_subcmd_matches > 0) + if (num_subcmd_matches > 0) { error_msg.assign("ambiguous command "); - else - error_msg.assign("invalid command "); - - error_msg.append("'"); - error_msg.append(std::string(GetCommandName())); - error_msg.append(" "); - error_msg.append(std::string(sub_command)); - error_msg.append("'."); + error_msg.append("'"); + error_msg.append(std::string(GetCommandName())); + error_msg.append(" "); + error_msg.append(std::string(sub_command)); + error_msg.append("'."); - if (num_subcmd_matches > 0) { error_msg.append(" Possible completions:"); for (const std::string &match : matches) { error_msg.append("\n\t"); error_msg.append(match); } + } else { + // Rather than simply complaining about the invalid (sub) command, + // try to offer some alternatives. + // This is especially useful for cases where the user types something + // seamingly trivial, such as `breakpoint foo`. + error_msg.assign( + llvm::Twine("'" + sub_command + "' is not a valid subcommand of \"" + + GetCommandName() + "\". Valid subcommands are " + + GetTopSubcommands(/*count=*/5) + ". Use \"help " + + GetCommandName() + "\" to find out more.") + .str()); } error_msg.append("\n"); result.AppendRawError(error_msg.c_str()); } +std::string CommandObjectMultiword::GetTopSubcommands(int count) { + if (m_subcommand_dict.empty()) + return "<NONE>"; + std::string buffer = "{"; + CommandMap::iterator pos; + for (pos = m_subcommand_dict.begin(); + pos != m_subcommand_dict.end() && count > 0; ++pos, --count) { + buffer.append("'"); + buffer.append(pos->first); + buffer.append("',"); + } + buffer.append("...}"); + return buffer; +} + void CommandObjectMultiword::GenerateHelpText(Stream &output_stream) { // First time through here, generate the help text for the object and push it // to the return result object as well diff --git a/lldb/test/Shell/Commands/command-breakpoint-col.test b/lldb/test/Shell/Commands/command-breakpoint-col.test index 65c1e220794303..fecb773d2b4c66 100644 --- a/lldb/test/Shell/Commands/command-breakpoint-col.test +++ b/lldb/test/Shell/Commands/command-breakpoint-col.test @@ -3,8 +3,10 @@ # RUN: %clang_host -g -O0 %S/Inputs/main.c -o %t.out # RUN: %lldb -b -o 'help breakpoint set' -o 'breakpoint set -f main.c -l 2 -u 21' %t.out | FileCheck %s --check-prefix HELP --check-prefix CHECK # RUN: %lldb -b -o 'help _regexp-break' -o 'b main.c:2:21' %t.out | FileCheck %s --check-prefix HELP-REGEX --check-prefix CHECK +# RUN: not %lldb -b -o 'breakpoint foo' %t.out -o exit 2>&1 | FileCheck %s --check-prefix ERROR-MSG # HELP: -u <column> ( --column <column> ) # HELP: Specifies the column number on which to set this breakpoint. # HELP-REGEX: _regexp-break <filename>:<linenum>:<colnum> # HELP-REGEX: main.c:12:21{{.*}}Break at line 12 and column 21 of main.c # CHECK: at main.c:2:21 +# ERROR-MSG: 'foo' is not a valid subcommand of "breakpoint". Valid subcommands are {'clear','command','delete','disable','enable',...}. Use "help breakpoint" to find out more. >From 8dfb266f7eed943f7d9c4bd00e09ef86ae1c5d6e Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Fri, 11 Oct 2024 11:30:06 -0400 Subject: [PATCH 2/3] addressed review comments: reword error msg and rename helper func --- .../lldb/Interpreter/CommandObjectMultiword.h | 2 +- .../Commands/CommandObjectMultiword.cpp | 32 +++++++++++-------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandObjectMultiword.h b/lldb/include/lldb/Interpreter/CommandObjectMultiword.h index cee118c3f454b5..c5e366e682600f 100644 --- a/lldb/include/lldb/Interpreter/CommandObjectMultiword.h +++ b/lldb/include/lldb/Interpreter/CommandObjectMultiword.h @@ -70,7 +70,7 @@ class CommandObjectMultiword : public CommandObject { return m_subcommand_dict; } - std::string GetTopSubcommands(int count); + std::string GetSubcommandsHintText(); CommandObject::CommandMap m_subcommand_dict; bool m_can_be_removed; diff --git a/lldb/source/Commands/CommandObjectMultiword.cpp b/lldb/source/Commands/CommandObjectMultiword.cpp index f7bb58187895b4..5450b60cb311b1 100644 --- a/lldb/source/Commands/CommandObjectMultiword.cpp +++ b/lldb/source/Commands/CommandObjectMultiword.cpp @@ -208,33 +208,37 @@ void CommandObjectMultiword::Execute(const char *args_string, error_msg.append(match); } } else { - // Rather than simply complaining about the invalid (sub) command, - // try to offer some alternatives. - // This is especially useful for cases where the user types something - // seamingly trivial, such as `breakpoint foo`. + // Try to offer some alternatives to help correct the command. error_msg.assign( llvm::Twine("'" + sub_command + "' is not a valid subcommand of \"" + - GetCommandName() + "\". Valid subcommands are " + - GetTopSubcommands(/*count=*/5) + ". Use \"help " + - GetCommandName() + "\" to find out more.") + GetCommandName() + "\"." + GetSubcommandsHintText() + + " Use \"help " + GetCommandName() + "\" to find out more.") .str()); } error_msg.append("\n"); result.AppendRawError(error_msg.c_str()); } -std::string CommandObjectMultiword::GetTopSubcommands(int count) { +std::string CommandObjectMultiword::GetSubcommandsHintText() { if (m_subcommand_dict.empty()) - return "<NONE>"; - std::string buffer = "{"; + return ""; + const size_t maxCount = 5; + size_t i = 0; + std::string buffer = " Valid subcommand"; + buffer.append(m_subcommand_dict.size() > 1 ? "s are:" : "is"); CommandMap::iterator pos; for (pos = m_subcommand_dict.begin(); - pos != m_subcommand_dict.end() && count > 0; ++pos, --count) { - buffer.append("'"); + pos != m_subcommand_dict.end() && i < maxCount; ++pos, ++i) { + buffer.append(" "); buffer.append(pos->first); - buffer.append("',"); + buffer.append(","); } - buffer.append("...}"); + if (i < m_subcommand_dict.size()) + buffer.append(" and others"); + else + buffer.pop_back(); + + buffer.append("."); return buffer; } >From b01e120f73996c96e7f8f8bee61bed72dab26a8f Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Sat, 12 Oct 2024 08:40:36 -0400 Subject: [PATCH 3/3] move tests to a new file. added test on nested commands, eg watchpoint set --- lldb/test/Shell/Commands/command-breakpoint-col.test | 2 -- .../Commands/command-wrong-subcommand-error-msg.test | 8 ++++++++ 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 lldb/test/Shell/Commands/command-wrong-subcommand-error-msg.test diff --git a/lldb/test/Shell/Commands/command-breakpoint-col.test b/lldb/test/Shell/Commands/command-breakpoint-col.test index fecb773d2b4c66..65c1e220794303 100644 --- a/lldb/test/Shell/Commands/command-breakpoint-col.test +++ b/lldb/test/Shell/Commands/command-breakpoint-col.test @@ -3,10 +3,8 @@ # RUN: %clang_host -g -O0 %S/Inputs/main.c -o %t.out # RUN: %lldb -b -o 'help breakpoint set' -o 'breakpoint set -f main.c -l 2 -u 21' %t.out | FileCheck %s --check-prefix HELP --check-prefix CHECK # RUN: %lldb -b -o 'help _regexp-break' -o 'b main.c:2:21' %t.out | FileCheck %s --check-prefix HELP-REGEX --check-prefix CHECK -# RUN: not %lldb -b -o 'breakpoint foo' %t.out -o exit 2>&1 | FileCheck %s --check-prefix ERROR-MSG # HELP: -u <column> ( --column <column> ) # HELP: Specifies the column number on which to set this breakpoint. # HELP-REGEX: _regexp-break <filename>:<linenum>:<colnum> # HELP-REGEX: main.c:12:21{{.*}}Break at line 12 and column 21 of main.c # CHECK: at main.c:2:21 -# ERROR-MSG: 'foo' is not a valid subcommand of "breakpoint". Valid subcommands are {'clear','command','delete','disable','enable',...}. Use "help breakpoint" to find out more. diff --git a/lldb/test/Shell/Commands/command-wrong-subcommand-error-msg.test b/lldb/test/Shell/Commands/command-wrong-subcommand-error-msg.test new file mode 100644 index 00000000000000..dd62964834672f --- /dev/null +++ b/lldb/test/Shell/Commands/command-wrong-subcommand-error-msg.test @@ -0,0 +1,8 @@ +# UNSUPPORTED: system-windows +# +# RUN: %clang_host -g -O0 %S/Inputs/main.c -o %t.out +# RUN: not %lldb -b -o 'breakpoint foo' %t.out -o exit 2>&1 | FileCheck %s --check-prefix BP-MSG +# RUN: not %lldb -b -o 'watchpoint set foo' %t.out -o exit 2>&1 | FileCheck %s --check-prefix WP-MSG +# CHECK: at main.c:2:21 +# BP-MSG: 'foo' is not a valid subcommand of "breakpoint". Valid subcommands are: clear, command, delete, disable, enable, and others. Use "help breakpoint" to find out more. +# WP-MSG: 'foo' is not a valid subcommand of "watchpoint set". Valid subcommands are: expression, variable. Use "help watchpoint set" to find out more. \ No newline at end of file _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits