https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/153695
Whenever an option would use something other than the first letter of the long option as the short option, Jim would capitalized the letter we picked as a mnemonic. This has often been mistaken for a typo and Jim wondered if we should stop doing this. During the discussion, David mentioned how this reminds him of the underline in menu bars when holding down alt. I suggested we do something similar in LLDB by underlying the letter in the description. Here's what that looks like for a subset of the `process attach` options: <img width="570" height="165" alt="Screenshot 2025-08-14 at 4 49 12 PM" src="https://github.com/user-attachments/assets/576caade-11d5-4595-9676-46d012257d42"; /> See https://discourse.llvm.org/t/should-we-remove-the-capital-letter-in-option-helps/87816 >From cc184031ead83e95a87912e72a69780ef8f71967 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Thu, 14 Aug 2025 14:44:05 -0700 Subject: [PATCH] [lldb] Underline short option letters as mnemonics Whenever an option would use something other than the first letter of the long option as the short option, Jim would capitalized the letter we picked as a mnemonic. This has often been mistaken for a typo and Jim wondered if we should stop doing this. During the discussion, David mentioned how this reminds him of the underline in menu bars when holding down alt. I suggested we do something similar in LLDB by underlying the letter in the description. See https://discourse.llvm.org/t/should-we-remove-the-capital-letter-in-option-helps/87816 --- lldb/include/lldb/Interpreter/Options.h | 4 +- .../Commands/CommandObjectDisassemble.cpp | 3 +- lldb/source/Commands/CommandObjectFrame.cpp | 3 +- lldb/source/Commands/CommandObjectTarget.cpp | 3 +- lldb/source/Commands/Options.td | 41 +++++++++++++------ lldb/source/Interpreter/CommandObject.cpp | 6 ++- lldb/source/Interpreter/Options.cpp | 13 +++--- 7 files changed, 49 insertions(+), 24 deletions(-) diff --git a/lldb/include/lldb/Interpreter/Options.h b/lldb/include/lldb/Interpreter/Options.h index 864bda6f24c8c..2d06605f5f0b0 100644 --- a/lldb/include/lldb/Interpreter/Options.h +++ b/lldb/include/lldb/Interpreter/Options.h @@ -85,10 +85,10 @@ class Options { void OutputFormattedUsageText(Stream &strm, const OptionDefinition &option_def, - uint32_t output_max_columns); + uint32_t output_max_columns, bool use_color); void GenerateOptionUsage(Stream &strm, CommandObject &cmd, - uint32_t screen_width); + uint32_t screen_width, bool use_color); bool SupportsLongOption(const char *long_option); diff --git a/lldb/source/Commands/CommandObjectDisassemble.cpp b/lldb/source/Commands/CommandObjectDisassemble.cpp index 70e687e19ac6d..f8e4ff9f08b6c 100644 --- a/lldb/source/Commands/CommandObjectDisassemble.cpp +++ b/lldb/source/Commands/CommandObjectDisassemble.cpp @@ -503,8 +503,9 @@ void CommandObjectDisassemble::DoExecute(Args &command, "\"disassemble\" arguments are specified as options.\n"); const int terminal_width = GetCommandInterpreter().GetDebugger().GetTerminalWidth(); + const bool use_color = GetCommandInterpreter().GetDebugger().GetUseColor(); GetOptions()->GenerateOptionUsage(result.GetErrorStream(), *this, - terminal_width); + terminal_width, use_color); return; } diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index 56926999d1678..7e58a95fae2c3 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -349,7 +349,8 @@ class CommandObjectFrameSelect : public CommandObjectParsed { command[0].c_str()); m_options.GenerateOptionUsage( result.GetErrorStream(), *this, - GetCommandInterpreter().GetDebugger().GetTerminalWidth()); + GetCommandInterpreter().GetDebugger().GetTerminalWidth(), + GetCommandInterpreter().GetDebugger().GetUseColor()); return; } diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index dbebbbd38093e..3ae08dec75e31 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -4075,7 +4075,8 @@ class CommandObjectTargetModulesLookup : public CommandObjectParsed { default: m_options.GenerateOptionUsage( result.GetErrorStream(), *this, - GetCommandInterpreter().GetDebugger().GetTerminalWidth()); + GetCommandInterpreter().GetDebugger().GetTerminalWidth(), + GetCommandInterpreter().GetDebugger().GetUseColor()); syntax_error = true; break; } diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 61acc40585976..ac2817d158a05 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -736,18 +736,35 @@ let Command = "process launch" in { } let Command = "process attach" in { - def process_attach_continue : Option<"continue", "c">, - Desc<"Immediately continue the process once attached.">; - def process_attach_plugin : Option<"plugin", "P">, Arg<"Plugin">, - Desc<"Name of the process plugin you want to use.">; - def process_attach_pid : Option<"pid", "p">, Group<1>, Arg<"Pid">, - Desc<"The process ID of an existing process to attach to.">; - def process_attach_name : Option<"name", "n">, Group<2>, Arg<"ProcessName">, - Desc<"The name of the process to attach to.">; - def process_attach_include_existing : Option<"include-existing", "i">, - Group<2>, Desc<"Include existing processes when doing attach -w.">; - def process_attach_waitfor : Option<"waitfor", "w">, Group<2>, - Desc<"Wait for the process with <process-name> to launch.">; + def process_attach_continue + : Option<"continue", "c">, + Desc<"Immediately ${ansi.underline}c${ansi.normal}ontinue the process " + "once attached.">; + def process_attach_plugin + : Option<"plugin", "P">, + Arg<"Plugin">, + Desc<"Name of the process ${ansi.underline}p${ansi.normal}lugin you " + "want to use.">; + def process_attach_pid : Option<"pid", "p">, + Group<1>, + Arg<"Pid">, + Desc<"The ${ansi.underline}p${ansi.normal}rocess ID " + "of an existing process to attach to.">; + def process_attach_name : Option<"name", "n">, + Group<2>, + Arg<"ProcessName">, + Desc<"The ${ansi.underline}n${ansi.normal}ame of " + "the process to attach to.">; + def process_attach_include_existing + : Option<"include-existing", "i">, + Group<2>, + Desc<"${ansi.underline}I${ansi.normal}nclude existing processes when " + "doing attach -w.">; + def process_attach_waitfor + : Option<"waitfor", "w">, + Group<2>, + Desc<"${ansi.underline}W${ansi.normal}ait for the process with " + "<process-name> to launch.">; } let Command = "process continue" in { diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp index 129646ebddb94..43e19b397ae1f 100644 --- a/lldb/source/Interpreter/CommandObject.cpp +++ b/lldb/source/Interpreter/CommandObject.cpp @@ -359,7 +359,8 @@ bool CommandObject::HelpTextContainsWord(llvm::StringRef search_word, StreamString usage_help; GetOptions()->GenerateOptionUsage( usage_help, *this, - GetCommandInterpreter().GetDebugger().GetTerminalWidth()); + GetCommandInterpreter().GetDebugger().GetTerminalWidth(), + GetCommandInterpreter().GetDebugger().GetUseColor()); if (!usage_help.Empty()) { llvm::StringRef usage_text = usage_help.GetString(); if (usage_text.contains_insensitive(search_word)) @@ -672,7 +673,8 @@ void CommandObject::GenerateHelpText(Stream &output_strm) { if (options != nullptr) { options->GenerateOptionUsage( output_strm, *this, - GetCommandInterpreter().GetDebugger().GetTerminalWidth()); + GetCommandInterpreter().GetDebugger().GetTerminalWidth(), + GetCommandInterpreter().GetDebugger().GetUseColor()); } llvm::StringRef long_help = GetHelpLong(); if (!long_help.empty()) { diff --git a/lldb/source/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp index 4cf68db466158..ec725428483ff 100644 --- a/lldb/source/Interpreter/Options.cpp +++ b/lldb/source/Interpreter/Options.cpp @@ -19,6 +19,7 @@ #include "lldb/Interpreter/CommandObject.h" #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Target/Target.h" +#include "lldb/Utility/AnsiTerminal.h" #include "lldb/Utility/DiagnosticsRendering.h" #include "lldb/Utility/StreamString.h" #include "llvm/ADT/STLExtras.h" @@ -261,7 +262,8 @@ Option *Options::GetLongOptions() { void Options::OutputFormattedUsageText(Stream &strm, const OptionDefinition &option_def, - uint32_t output_max_columns) { + uint32_t output_max_columns, + bool use_color) { std::string actual_text; if (option_def.validator) { const char *condition = option_def.validator->ShortConditionString(); @@ -278,7 +280,7 @@ void Options::OutputFormattedUsageText(Stream &strm, if (static_cast<uint32_t>(actual_text.length() + strm.GetIndentLevel()) < output_max_columns) { // Output it as a single line. - strm.Indent(actual_text); + strm.Indent(ansi::FormatAnsiTerminalCodes(actual_text, use_color)); strm.EOL(); } else { // We need to break it up into multiple lines. @@ -312,7 +314,8 @@ void Options::OutputFormattedUsageText(Stream &strm, strm.Indent(); assert(start < final_end); assert(start + sub_len <= final_end); - strm.Write(actual_text.c_str() + start, sub_len); + strm.PutCString(ansi::FormatAnsiTerminalCodes( + llvm::StringRef(actual_text.c_str() + start, sub_len), use_color)); start = end + 1; } strm.EOL(); @@ -385,7 +388,7 @@ static bool PrintOption(const OptionDefinition &opt_def, } void Options::GenerateOptionUsage(Stream &strm, CommandObject &cmd, - uint32_t screen_width) { + uint32_t screen_width, bool use_color) { auto opt_defs = GetDefinitions(); const uint32_t save_indent_level = strm.GetIndentLevel(); llvm::StringRef name = cmd.GetCommandName(); @@ -527,7 +530,7 @@ void Options::GenerateOptionUsage(Stream &strm, CommandObject &cmd, strm.IndentMore(5); if (opt_def.usage_text) - OutputFormattedUsageText(strm, opt_def, screen_width); + OutputFormattedUsageText(strm, opt_def, screen_width, use_color); if (!opt_def.enum_values.empty()) { strm.Indent(); strm.Printf("Values: "); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
