-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[lldb] Add a simplified syntax for underlying command options (NFC) #155694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb] Add a simplified syntax for underlying command options (NFC) #155694
Conversation
This PR updates the tablegen emitter for command options to support a simplified syntax to underline the mnemonic. Previously, you had to write `${ansi.underline}<L>${ansi.normal}`, where `<L>` is the mnemonic. This really hurt the readability of the description. With this PR, you can write `${<L>}` instead.
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesThis PR updates the tablegen emitter for command options to support a simplified syntax to underline the mnemonic. Previously, you had to write Full diff: https://github.com/llvm/llvm-project/pull/155694.diff 2 Files Affected:
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 2aa0b10ac34e5..e61df1342ce9f 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -741,35 +741,32 @@ let Command = "process launch" in {
}
let Command = "process attach" in {
- 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_continue : Option<"continue", "c">,
+ Desc<"Immediately ${c}ontinue the process "
+ "once attached.">;
+ def process_attach_plugin : Option<"plugin", "P">,
+ Arg<"Plugin">,
+ Desc<"Name of the process ${p}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 "
+ Desc<"The ${p}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 "
+ Desc<"The ${n}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 "
+ Desc<"${I}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.">;
+ def process_attach_waitfor : Option<"waitfor", "w">,
+ Group<2>,
+ Desc<"${W}ait for the process with "
+ "<process-name> to launch.">;
}
let Command = "process continue" in {
diff --git a/lldb/utils/TableGen/LLDBOptionDefEmitter.cpp b/lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
index 2507910d8a97a..b39622240693f 100644
--- a/lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
+++ b/lldb/utils/TableGen/LLDBOptionDefEmitter.cpp
@@ -23,6 +23,31 @@ using namespace llvm;
using namespace lldb_private;
namespace {
+/// Parses curly braces and replaces them with ANSI underline formatting.
+inline std::string underline(llvm::StringRef Str) {
+ llvm::StringRef OpeningHead, OpeningTail, ClosingHead, ClosingTail;
+ std::string Result;
+ llvm::raw_string_ostream Stream(Result);
+ while (!Str.empty()) {
+ // Find the opening brace.
+ std::tie(OpeningHead, OpeningTail) = Str.split("${");
+ Stream << OpeningHead;
+
+ // No opening brace: we're done.
+ if (OpeningHead == Str && OpeningTail.empty())
+ break;
+
+ // Find the closing brace.
+ std::tie(ClosingHead, ClosingTail) = OpeningTail.split('}');
+ assert(!ClosingTail.empty() &&
+ "unmatched curly braces in command option description");
+
+ Stream << "${ansi.underline}" << ClosingHead << "${ansi.normal}";
+ Str = ClosingTail;
+ }
+ return Result;
+}
+
struct CommandOption {
std::vector<std::string> GroupsArg;
bool Required = false;
@@ -68,7 +93,7 @@ struct CommandOption {
Completions = Option->getValueAsListOfStrings("Completions");
if (auto D = Option->getValue("Description"))
- Description = D->getValue()->getAsUnquotedString();
+ Description = underline(D->getValue()->getAsUnquotedString());
}
};
} // namespace
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed it would be nice to have a short-hand syntax for this. Do we want to claim ${}
for underlining though? Or could have a slightly different short-hand notation that we could then also extend to making things bold, italic, colored, etc. E.g., ${u:<blah>}
. Of course then we'd have to come up with some sort of escaping mechanism. Wdyt?
We can always unclaim it later. This is a DSL we control and there are no external users. If we later want more, or want to align to another part of LLVM doing similar things, there's no problem in doing so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably could do this in TableGen itself, but it would be rather awkward no doubt.
I like that ${N}
is grepable if we need to find all these again.
Don't people use these in their .lldbinit? But if we're free to change it later down the line, that definitely alleviates my concern |
Ah this isn't updating the format language, just the format that we use in the tablegen definitions. Nvm then |
Yup, exactly, this is strictly limited to our TableGen/DSL backend. |
Ugh clicked the wrong button. |
44c410c
to
6dd12a7
Compare
Co-authored-by: Michael Buch <michaelbuch12@gmail.com>
This PR updates the tablegen emitter for command options to support a simplified syntax to underline the mnemonic. Previously, you had to write
${ansi.underline}<L>${ansi.normal}
, where<L>
is the mnemonic. This really hurt the readability of the description. With this PR, you can write${<L>}
instead.