Skip to content

[lldb] Highlight "note:" in CommandReturnObject #114610

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

Merged
merged 1 commit into from
Nov 2, 2024

Conversation

JDevlieghere
Copy link
Member

We have helpers to emit warnings and errors. Do the same thing for notes to they stand out more.

@JDevlieghere
Copy link
Member Author

Motivated by #114608

@llvmbot
Copy link
Member

llvmbot commented Nov 1, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

We have helpers to emit warnings and errors. Do the same thing for notes to they stand out more.


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

3 Files Affected:

  • (modified) lldb/include/lldb/Interpreter/CommandReturnObject.h (+5)
  • (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+5-6)
  • (modified) lldb/source/Interpreter/CommandReturnObject.cpp (+24)
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index a491a6c1535b11..14f2c4a62f7cbd 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -110,6 +110,11 @@ class CommandReturnObject {
   void AppendMessageWithFormat(const char *format, ...)
       __attribute__((format(printf, 2, 3)));
 
+  void AppendNote(llvm::StringRef in_string);
+
+  void AppendNoteWithFormat(const char *format, ...)
+      __attribute__((format(printf, 2, 3)));
+
   void AppendWarning(llvm::StringRef in_string);
 
   void AppendWarningWithFormat(const char *format, ...)
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 76bed100dc7291..dc0f98ed06f7fe 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -121,8 +121,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
       if (note_shown)
         return;
 
-      result.GetOutputStream()
-          << "note: object description requested, but type doesn't implement "
+      result.AppendNote(
+          << "object description requested, but type doesn't implement "
              "a custom object description. Consider using \"p\" instead of "
              "\"po\" (this note will only be shown once per debug session).\n";
       note_shown = true;
@@ -164,8 +164,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
         StringRef flags;
         if (args.HasArgs())
           flags = args.GetArgString();
-        result.AppendMessageWithFormatv("note: ran `frame variable {0}{1}`",
-                                        flags, expr);
+        result.AppendNoteWithFormatv("ran `frame variable {0}{1}`", flags,
+                                     expr);
       }
 
       dump_val_object(*valobj_sp);
@@ -224,8 +224,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
       StringRef flags;
       if (args.HasArgs())
         flags = args.GetArgStringWithDelimiter();
-      result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
-                                      expr);
+      result.AppendNoteWithFormatv("ran `expression {0}{1}`", flags, expr);
     }
 
     if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp
index 94f5ff608b2aea..2776efbb5ee36d 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -27,6 +27,12 @@ static llvm::raw_ostream &warning(Stream &strm) {
          << "warning: ";
 }
 
+static llvm::raw_ostream &note(Stream &strm) {
+  return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Note,
+                         llvm::ColorMode::Enable)
+         << "note: ";
+}
+
 static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) {
   bool add_newline = false;
   if (!s.empty()) {
@@ -74,6 +80,18 @@ void CommandReturnObject::AppendMessageWithFormat(const char *format, ...) {
   GetOutputStream() << sstrm.GetString();
 }
 
+void CommandReturnObject::AppendNoteWithFormat(const char *format, ...) {
+  if (!format)
+    return;
+  va_list args;
+  va_start(args, format);
+  StreamString sstrm;
+  sstrm.PrintfVarArg(format, args);
+  va_end(args);
+
+  note(GetOutputStream()) << sstrm.GetString();
+}
+
 void CommandReturnObject::AppendWarningWithFormat(const char *format, ...) {
   if (!format)
     return;
@@ -92,6 +110,12 @@ void CommandReturnObject::AppendMessage(llvm::StringRef in_string) {
   GetOutputStream() << in_string.rtrim() << '\n';
 }
 
+void CommandReturnObject::AppendNote(llvm::StringRef in_string) {
+  if (in_string.empty())
+    return;
+  note(GetOutputStream()) << in_string.rtrim() << '\n';
+}
+
 void CommandReturnObject::AppendWarning(llvm::StringRef in_string) {
   if (in_string.empty())
     return;

void AppendNote(llvm::StringRef in_string);

void AppendNoteWithFormat(const char *format, ...)
__attribute__((format(printf, 2, 3)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, didn't know about __attribute__((format(printf

@@ -27,6 +27,12 @@ static llvm::raw_ostream &warning(Stream &strm) {
<< "warning: ";
}

static llvm::raw_ostream &note(Stream &strm) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since note is a function should it be capitalized to follow the LLDB style?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually error and warning are not capitalized, I guess we can either keep all three lower case or capitalize all of them, up to you

Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to say this is not the right way to do this, but I suppose there isn't really a better mechanism available, since this isn't an error. Otherwise you'd be better off joining a diagnostic error with the note so the UI layer can choose how to render the note.

We have helpers to emit warnings and errors. Do the same thing for notes
to they stand out more.
@JDevlieghere JDevlieghere merged commit 79178ca into llvm:main Nov 2, 2024
5 of 7 checks passed
@JDevlieghere JDevlieghere deleted the notes branch November 2, 2024 15:52
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this pull request Nov 3, 2024
We have helpers to emit warnings and errors. Do the same thing for notes
to they stand out more.
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
We have helpers to emit warnings and errors. Do the same thing for notes
to they stand out more.
kastiglione pushed a commit to swiftlang/llvm-project that referenced this pull request Mar 15, 2025
We have helpers to emit warnings and errors. Do the same thing for notes
to they stand out more.

(cherry picked from commit 79178ca)
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.

5 participants