Skip to content

Conversation

JDevlieghere
Copy link
Member

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.

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.
@llvmbot
Copy link
Member

llvmbot commented Aug 27, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

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}&lt;L&gt;${ansi.normal}, where &lt;L&gt; is the mnemonic. This really hurt the readability of the description. With this PR, you can write ${&lt;L&gt;} instead.


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

2 Files Affected:

  • (modified) lldb/source/Commands/Options.td (+14-17)
  • (modified) lldb/utils/TableGen/LLDBOptionDefEmitter.cpp (+26-1)
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

Copy link
Member

@Michael137 Michael137 left a 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?

@DavidSpickett
Copy link
Collaborator

Agreed it would be nice to have a short-hand syntax for this. Do we want to claim ${} for underlining though?

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.

Copy link
Collaborator

@DavidSpickett DavidSpickett left a 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.

@Michael137
Copy link
Member

Agreed it would be nice to have a short-hand syntax for this. Do we want to claim ${} for underlining though?

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.

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

@Michael137
Copy link
Member

Ah this isn't updating the format language, just the format that we use in the tablegen definitions. Nvm then

@JDevlieghere
Copy link
Member Author

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.

@JDevlieghere
Copy link
Member Author

Ugh clicked the wrong button.

@JDevlieghere JDevlieghere reopened this Aug 28, 2025
@JDevlieghere JDevlieghere force-pushed the tablegen-option-emitter-syntax branch from 44c410c to 6dd12a7 Compare August 28, 2025 18:05
Co-authored-by: Michael Buch <michaelbuch12@gmail.com>
@JDevlieghere JDevlieghere merged commit 81d6b20 into llvm:main Aug 29, 2025
9 checks passed
@JDevlieghere JDevlieghere deleted the tablegen-option-emitter-syntax branch August 29, 2025 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants