[lldb][NFC] Replace prefix/suffix variables with ColorSetting - #210671
Open
Teemperor wants to merge 1 commit into
Open
[lldb][NFC] Replace prefix/suffix variables with ColorSetting#210671Teemperor wants to merge 1 commit into
Teemperor wants to merge 1 commit into
Conversation
We use pairs of prefix/suffix strings across our code base to represent the ANSI color codes emitted before/after a highlighted word. This patch replaces these pairs with a single ColorSettings class. The motivation is just to get rid of all the duplicated prefix/suffix variables and functions everywhere.
|
@llvm/pr-subscribers-lldb Author: Raphael Isemann (Teemperor) ChangesWe use pairs of prefix/suffix strings across our code base to represent the ANSI color codes emitted before/after a highlighted word. This patch replaces these pairs with a single ColorSettings class. The motivation is just to get rid of all the duplicated prefix/suffix variables and functions everywhere. Patch is 22.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/210671.diff 13 Files Affected:
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 2501eeabfe506..5138f67a098c8 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -31,6 +31,7 @@
#include "lldb/Target/Platform.h"
#include "lldb/Target/TargetList.h"
#include "lldb/Utility/Broadcaster.h"
+#include "lldb/Utility/ColorSetting.h"
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Status.h"
@@ -310,9 +311,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
llvm::StringRef GetPrompt() const;
- llvm::StringRef GetPromptAnsiPrefix() const;
-
- llvm::StringRef GetPromptAnsiSuffix() const;
+ ColorSetting GetPromptColor() const;
void SetPrompt(llvm::StringRef p);
void SetPrompt(const char *) = delete;
@@ -340,23 +339,15 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
llvm::StringRef GetSeparator() const;
bool SetSeparator(llvm::StringRef s);
- llvm::StringRef GetShowProgressAnsiPrefix() const;
-
- llvm::StringRef GetShowProgressAnsiSuffix() const;
+ ColorSetting GetShowProgressColor() const;
- llvm::StringRef GetDisabledAnsiPrefix() const;
-
- llvm::StringRef GetDisabledAnsiSuffix() const;
+ ColorSetting GetDisabledColor() const;
AutosuggestionMode GetAutosuggestionMode() const;
- llvm::StringRef GetAutosuggestionAnsiPrefix() const;
-
- llvm::StringRef GetAutosuggestionAnsiSuffix() const;
+ ColorSetting GetAutosuggestionColor() const;
- llvm::StringRef GetRegexMatchAnsiPrefix() const;
-
- llvm::StringRef GetRegexMatchAnsiSuffix() const;
+ ColorSetting GetRegexMatchColor() const;
bool GetShowDontUsePoHint() const;
@@ -370,9 +361,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
lldb::StopShowColumn GetStopShowColumn() const;
- llvm::StringRef GetStopShowColumnAnsiPrefix() const;
-
- llvm::StringRef GetStopShowColumnAnsiSuffix() const;
+ ColorSetting GetStopShowColumnColor() const;
uint64_t GetStopSourceLineCount(bool before) const;
@@ -380,9 +369,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
uint64_t GetDisassemblyLineCount() const;
- llvm::StringRef GetStopShowLineMarkerAnsiPrefix() const;
-
- llvm::StringRef GetStopShowLineMarkerAnsiSuffix() const;
+ ColorSetting GetStopShowLineMarkerColor() const;
bool GetAutoOneLineSummaries() const;
@@ -669,6 +656,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
friend class ProgressManager;
friend class Statusline;
+ /// Read the pair of ANSI color-code settings at the given property indices
+ /// and return them as a single ColorSetting.
+ ColorSetting GetColorProperty(uint32_t prefix_idx, uint32_t suffix_idx) const;
+
/// Report progress events.
///
/// Progress events will be delivered to any debuggers that have listeners
diff --git a/lldb/include/lldb/Utility/ColorSetting.h b/lldb/include/lldb/Utility/ColorSetting.h
new file mode 100644
index 0000000000000..55b7041f6cdb9
--- /dev/null
+++ b/lldb/include/lldb/Utility/ColorSetting.h
@@ -0,0 +1,42 @@
+//===-- ColorSetting.h ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_COLORSETTING_H
+#define LLDB_UTILITY_COLORSETTING_H
+
+#include "llvm/ADT/StringRef.h"
+
+namespace lldb_private {
+
+class Stream;
+
+/// A pair of ANSI terminal escape sequences used to colorize a piece of text.
+///
+/// The \c prefix is emitted immediately before the text and the \c suffix
+/// immediately after it (typically a reset code) to restore the previous
+/// terminal appearance.
+class ColorSetting {
+public:
+ ColorSetting() = default;
+ ColorSetting(llvm::StringRef prefix, llvm::StringRef suffix)
+ : m_prefix(prefix), m_suffix(suffix) {}
+
+ llvm::StringRef GetPrefix() const { return m_prefix; }
+ llvm::StringRef GetSuffix() const { return m_suffix; }
+
+ /// Write \a str to \a s wrapped in this setting's ANSI color codes.
+ void render(Stream &s, llvm::StringRef str) const;
+
+private:
+ llvm::StringRef m_prefix;
+ llvm::StringRef m_suffix;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_UTILITY_COLORSETTING_H
diff --git a/lldb/include/lldb/Utility/Stream.h b/lldb/include/lldb/Utility/Stream.h
index 5b872f14d0134..41b2ee5fdb061 100644
--- a/lldb/include/lldb/Utility/Stream.h
+++ b/lldb/include/lldb/Utility/Stream.h
@@ -9,6 +9,7 @@
#ifndef LLDB_UTILITY_STREAM_H
#define LLDB_UTILITY_STREAM_H
+#include "lldb/Utility/ColorSetting.h"
#include "lldb/Utility/Flags.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
@@ -36,13 +37,11 @@ class Stream {
/// Struct to store information for color highlighting in the stream.
struct HighlightSettings {
llvm::StringRef pattern; ///< Regex pattern for highlighting.
- llvm::StringRef prefix; ///< ANSI color code to start colorization.
- llvm::StringRef suffix; ///< ANSI color code to end colorization.
+ ColorSetting color; ///< ANSI color codes wrapping each match.
bool ignore_case = false; ///< Whether to match case-insensitively.
- HighlightSettings(llvm::StringRef p, llvm::StringRef pre,
- llvm::StringRef suf, bool ic = false)
- : pattern(p), prefix(pre), suffix(suf), ignore_case(ic) {}
+ HighlightSettings(llvm::StringRef p, ColorSetting color, bool ic = false)
+ : pattern(p), color(color), ignore_case(ic) {}
};
/// Utility class for counting the bytes that were written to a stream in a
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp
index ec8099fa3e94a..66c4d0f3dd1c3 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -928,9 +928,8 @@ void Breakpoint::GetDescription(Stream *s, lldb::DescriptionLevel level,
const bool dim_breakpoint_description =
!IsEnabled() && s->AsRawOstream().colors_enabled();
if (dim_breakpoint_description)
- s->Printf("%s", ansi::FormatAnsiTerminalCodes(
- GetTarget().GetDebugger().GetDisabledAnsiPrefix())
- .c_str());
+ s->PutCString(ansi::FormatAnsiTerminalCodes(
+ GetTarget().GetDebugger().GetDisabledColor().GetPrefix()));
if (!m_kind_description.empty()) {
if (level == eDescriptionLevelBrief) {
@@ -954,9 +953,8 @@ void Breakpoint::GetDescription(Stream *s, lldb::DescriptionLevel level,
}
// Reset the colors back to normal if they were previously greyed out.
if (dim_breakpoint_description)
- s->Printf("%s", ansi::FormatAnsiTerminalCodes(
- GetTarget().GetDebugger().GetDisabledAnsiSuffix())
- .c_str());
+ s->PutCString(ansi::FormatAnsiTerminalCodes(
+ GetTarget().GetDebugger().GetDisabledColor().GetSuffix()));
}
void Breakpoint::GetDescriptionForType(Stream *s, lldb::DescriptionLevel level,
diff --git a/lldb/source/Commands/CommandObjectApropos.cpp b/lldb/source/Commands/CommandObjectApropos.cpp
index 5f8719f6b5b8f..5b1ea868b4218 100644
--- a/lldb/source/Commands/CommandObjectApropos.cpp
+++ b/lldb/source/Commands/CommandObjectApropos.cpp
@@ -43,8 +43,7 @@ void CommandObjectApropos::DoExecute(Args &args, CommandReturnObject &result) {
Debugger &dbg = GetDebugger();
if (dbg.GetUseColor()) {
escaped_search_word = llvm::Regex::escape(search_word);
- highlight.emplace(escaped_search_word, dbg.GetRegexMatchAnsiPrefix(),
- dbg.GetRegexMatchAnsiSuffix(), true);
+ highlight.emplace(escaped_search_word, dbg.GetRegexMatchColor(), true);
}
// Find all commands matching the search word.
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index 4ef3a6fe82115..b536c47db3288 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1595,9 +1595,8 @@ static uint32_t LookupSymbolInModule(CommandInterpreter &interpreter,
DumpFullpath(strm, &module->GetFileSpec(), 0);
strm.PutCString(":\n");
strm.IndentMore();
- Stream::HighlightSettings settings(
- name, interpreter.GetDebugger().GetRegexMatchAnsiPrefix(),
- interpreter.GetDebugger().GetRegexMatchAnsiSuffix());
+ ColorSetting regex_color = interpreter.GetDebugger().GetRegexMatchColor();
+ Stream::HighlightSettings settings(name, regex_color);
for (uint32_t i = 0; i < num_matches; ++i) {
const Symbol *symbol = symtab->SymbolAtIndex(match_indexes[i]);
if (symbol) {
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 170bef020e3ea..e0370a030da28 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -422,16 +422,17 @@ llvm::StringRef Debugger::GetPrompt() const {
idx, g_debugger_properties[idx].default_cstr_value);
}
-llvm::StringRef Debugger::GetPromptAnsiPrefix() const {
- const uint32_t idx = ePropertyPromptAnsiPrefix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
+ColorSetting Debugger::GetColorProperty(uint32_t prefix_idx,
+ uint32_t suffix_idx) const {
+ return {
+ GetPropertyAtIndexAs<llvm::StringRef>(
+ prefix_idx, g_debugger_properties[prefix_idx].default_cstr_value),
+ GetPropertyAtIndexAs<llvm::StringRef>(
+ suffix_idx, g_debugger_properties[suffix_idx].default_cstr_value)};
}
-llvm::StringRef Debugger::GetPromptAnsiSuffix() const {
- const uint32_t idx = ePropertyPromptAnsiSuffix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
+ColorSetting Debugger::GetPromptColor() const {
+ return GetColorProperty(ePropertyPromptAnsiPrefix, ePropertyPromptAnsiSuffix);
}
void Debugger::SetPrompt(llvm::StringRef p) {
@@ -566,16 +567,9 @@ bool Debugger::SetShowProgress(bool show_progress) {
return SetPropertyAtIndex(idx, show_progress);
}
-llvm::StringRef Debugger::GetShowProgressAnsiPrefix() const {
- const uint32_t idx = ePropertyShowProgressAnsiPrefix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
-}
-
-llvm::StringRef Debugger::GetShowProgressAnsiSuffix() const {
- const uint32_t idx = ePropertyShowProgressAnsiSuffix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
+ColorSetting Debugger::GetShowProgressColor() const {
+ return GetColorProperty(ePropertyShowProgressAnsiPrefix,
+ ePropertyShowProgressAnsiSuffix);
}
bool Debugger::GetShowStatusline() const {
@@ -602,16 +596,9 @@ llvm::StringRef Debugger::GetSeparator() const {
idx, g_debugger_properties[idx].default_cstr_value);
}
-llvm::StringRef Debugger::GetDisabledAnsiPrefix() const {
- const uint32_t idx = ePropertyShowDisabledAnsiPrefix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
-}
-
-llvm::StringRef Debugger::GetDisabledAnsiSuffix() const {
- const uint32_t idx = ePropertyShowDisabledAnsiSuffix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
+ColorSetting Debugger::GetDisabledColor() const {
+ return GetColorProperty(ePropertyShowDisabledAnsiPrefix,
+ ePropertyShowDisabledAnsiSuffix);
}
bool Debugger::SetSeparator(llvm::StringRef s) {
@@ -628,28 +615,14 @@ AutosuggestionMode Debugger::GetAutosuggestionMode() const {
g_debugger_properties[idx].default_uint_value));
}
-llvm::StringRef Debugger::GetAutosuggestionAnsiPrefix() const {
- const uint32_t idx = ePropertyShowAutosuggestionAnsiPrefix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
-}
-
-llvm::StringRef Debugger::GetAutosuggestionAnsiSuffix() const {
- const uint32_t idx = ePropertyShowAutosuggestionAnsiSuffix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
-}
-
-llvm::StringRef Debugger::GetRegexMatchAnsiPrefix() const {
- const uint32_t idx = ePropertyShowRegexMatchAnsiPrefix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
+ColorSetting Debugger::GetAutosuggestionColor() const {
+ return GetColorProperty(ePropertyShowAutosuggestionAnsiPrefix,
+ ePropertyShowAutosuggestionAnsiSuffix);
}
-llvm::StringRef Debugger::GetRegexMatchAnsiSuffix() const {
- const uint32_t idx = ePropertyShowRegexMatchAnsiSuffix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
+ColorSetting Debugger::GetRegexMatchColor() const {
+ return GetColorProperty(ePropertyShowRegexMatchAnsiPrefix,
+ ePropertyShowRegexMatchAnsiSuffix);
}
bool Debugger::GetShowDontUsePoHint() const {
@@ -692,28 +665,14 @@ StopShowColumn Debugger::GetStopShowColumn() const {
g_debugger_properties[idx].default_uint_value));
}
-llvm::StringRef Debugger::GetStopShowColumnAnsiPrefix() const {
- const uint32_t idx = ePropertyStopShowColumnAnsiPrefix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
-}
-
-llvm::StringRef Debugger::GetStopShowColumnAnsiSuffix() const {
- const uint32_t idx = ePropertyStopShowColumnAnsiSuffix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
-}
-
-llvm::StringRef Debugger::GetStopShowLineMarkerAnsiPrefix() const {
- const uint32_t idx = ePropertyStopShowLineMarkerAnsiPrefix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
+ColorSetting Debugger::GetStopShowColumnColor() const {
+ return GetColorProperty(ePropertyStopShowColumnAnsiPrefix,
+ ePropertyStopShowColumnAnsiSuffix);
}
-llvm::StringRef Debugger::GetStopShowLineMarkerAnsiSuffix() const {
- const uint32_t idx = ePropertyStopShowLineMarkerAnsiSuffix;
- return GetPropertyAtIndexAs<llvm::StringRef>(
- idx, g_debugger_properties[idx].default_cstr_value);
+ColorSetting Debugger::GetStopShowLineMarkerColor() const {
+ return GetColorProperty(ePropertyStopShowLineMarkerAnsiPrefix,
+ ePropertyStopShowLineMarkerAnsiSuffix);
}
uint64_t Debugger::GetStopSourceLineCount(bool before) const {
diff --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 0bb8b58f24bff..ae65c6c5d7df1 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -270,10 +270,11 @@ IOHandlerEditline::IOHandlerEditline(
m_editline_up->SetSuggestionCallback([this](llvm::StringRef line) {
return this->SuggestionCallback(line);
});
- m_editline_up->SetSuggestionAnsiPrefix(ansi::FormatAnsiTerminalCodes(
- debugger.GetAutosuggestionAnsiPrefix()));
- m_editline_up->SetSuggestionAnsiSuffix(ansi::FormatAnsiTerminalCodes(
- debugger.GetAutosuggestionAnsiSuffix()));
+ ColorSetting color = debugger.GetAutosuggestionColor();
+ m_editline_up->SetSuggestionAnsiPrefix(
+ ansi::FormatAnsiTerminalCodes(color.GetPrefix()));
+ m_editline_up->SetSuggestionAnsiSuffix(
+ ansi::FormatAnsiTerminalCodes(color.GetSuffix()));
}
// See if the delegate supports fixing indentation
const char *indent_chars = delegate.IOHandlerGetFixIndentationCharacters();
@@ -487,10 +488,11 @@ bool IOHandlerEditline::SetPrompt(llvm::StringRef prompt) {
#if LLDB_ENABLE_LIBEDIT
if (m_editline_up) {
m_editline_up->SetPrompt(m_prompt.empty() ? nullptr : m_prompt.c_str());
+ ColorSetting color = m_debugger.GetPromptColor();
m_editline_up->SetPromptAnsiPrefix(
- ansi::FormatAnsiTerminalCodes(m_debugger.GetPromptAnsiPrefix()));
+ ansi::FormatAnsiTerminalCodes(color.GetPrefix()));
m_editline_up->SetPromptAnsiSuffix(
- ansi::FormatAnsiTerminalCodes(m_debugger.GetPromptAnsiSuffix()));
+ ansi::FormatAnsiTerminalCodes(color.GetSuffix()));
}
#endif
return true;
@@ -502,10 +504,11 @@ bool IOHandlerEditline::SetUseColor(bool use_color) {
#if LLDB_ENABLE_LIBEDIT
if (m_editline_up) {
m_editline_up->UseColor(use_color);
- m_editline_up->SetSuggestionAnsiPrefix(ansi::FormatAnsiTerminalCodes(
- m_debugger.GetAutosuggestionAnsiPrefix()));
- m_editline_up->SetSuggestionAnsiSuffix(ansi::FormatAnsiTerminalCodes(
- m_debugger.GetAutosuggestionAnsiSuffix()));
+ ColorSetting color = m_debugger.GetAutosuggestionColor();
+ m_editline_up->SetSuggestionAnsiPrefix(
+ ansi::FormatAnsiTerminalCodes(color.GetPrefix()));
+ m_editline_up->SetSuggestionAnsiSuffix(
+ ansi::FormatAnsiTerminalCodes(color.GetSuffix()));
}
#endif
return true;
diff --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp
index 5ffedcc86ca1f..3e52e5749443a 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -30,6 +30,7 @@
#include "lldb/Utility/Log.h"
#include "lldb/Utility/RegularExpression.h"
#include "lldb/Utility/Stream.h"
+#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/SupportFile.h"
#include "lldb/lldb-enumerations.h"
@@ -272,11 +273,10 @@ size_t SourceManager::DisplaySourceLinesWithLineNumbersUsingLastFile(
auto debugger_sp = m_debugger_wp.lock();
if (should_show_stop_line_with_ansi(debugger_sp)) {
- current_line_highlight = ansi::FormatAnsiTerminalCodes(
- (debugger_sp->GetStopShowLineMarkerAnsiPrefix() +
- current_line_highlight +
- debugger_sp->GetStopShowLineMarkerAnsiSuffix())
- .str());
+ StreamString highlight_stream;
+ debugger_sp->GetStopShowLineMarkerColor().render(
+ highlight_stream, current_line_highlight);
+ current_line_highlight = highlight_stream.GetString().str();
}
s->Printf("%s%s %-4u\t", prefix.c_str(), current_line_highlight.c_str(),
@@ -712,9 +712,10 @@ size_t SourceManager::File::DisplaySourceLines(
// If we should mark the stop column with color codes, then copy the prefix
// and suffix to our color style.
- if (should_show_stop_column_with_ansi(debugger_sp))
- style.selected.Set(debugger_sp->GetStopShowColumnAnsiPrefix(),
- debugger_sp->GetStopShowColumnAnsiSuffix());
+ if (should_show_stop_column_with_ansi(debugger_sp)) {
+ ColorSetting color = debugger_sp->GetStopShowColumnColor();
+ style.selected.Set(color.GetPrefix(), color.GetSuffix());
+ }
HighlighterManager mgr;
std::string path =
diff --git a/lldb/source/Utility/CMakeLists.txt b/lldb/source/Utility/CMakeLists.txt
index c0d0a42367b26..5679a2a9ca2de 100644
--- a/lldb/source/Utility/CMakeLists.txt
+++ b/lldb/source/Utility/CMakeLists.txt
@@ -30,6 +30,7 @@ add_lldb_library(lldbUtility NO_INTERNAL_DEPENDENCIES
Baton.cpp
Broadcaster.cpp
Checksum.cpp
+ ColorSetting.cpp
CompletionRequest.cpp
Connection.cpp
ConstString.cpp
diff --git a/lldb/source/Utility/ColorSetting.cpp b/lldb/source/Utility/ColorSetting.cpp
new file mode 100644
index 0000000000000..a957e50fe0ed7
--- /dev/null
+++ b/lldb/source/Utility/ColorSetting.cpp
@@ -0,0 +1,22 @@
+//===-- ColorSetting.cpp --------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Utility/ColorSetting.h"
+
+#include "lldb/Utility/AnsiTerminal.h"
+#include "lldb/Utility/Stream.h"
+
+using namespace lldb_private;
+
+void ColorSetting::render(Stream &s, llvm::StringRef str) const {
+ if (!m_prefix...
[truncated]
|
| llvm::StringRef GetSuffix() const { return m_suffix; } | ||
|
|
||
| /// Write \a str to \a s wrapped in this setting's ANSI color codes. | ||
| void render(Stream &s, llvm::StringRef str) const; |
Contributor
There was a problem hiding this comment.
There's a buggy early return (predating this PR) that makes me think that this class could have this method removed, and then have a companion RAII class:
class RenderColorSettingRAII {
RenderColorSettingRAII(ColorSetting&, Stream &, bool should_color = true);
};
Contributor
There was a problem hiding this comment.
There's a buggy early return
Do you mean something in this PR's diff or elsewhere?
I guess it would be something like emitting the prefix but early returning before the suffix is emitted.
Contributor
There was a problem hiding this comment.
Do you mean something in this PR's diff or elsewhere?
it's not in the diff, but it can be seen in this PR's context: see lldb/source/Breakpoint/Breakpoint.cpp:937, a few files below.
| debugger_sp->GetStopShowColumnAnsiSuffix()); | ||
| if (should_show_stop_column_with_ansi(debugger_sp)) { | ||
| ColorSetting color = debugger_sp->GetStopShowColumnColor(); | ||
| style.selected.Set(color.GetPrefix(), color.GetSuffix()); |
Contributor
There was a problem hiding this comment.
For a follow up, ColorSetting looks a lot like HighlightStyle::ColorStyle. I'm not sure you can share the type well enough to make it worth it, but worth looking at whether Set(ColorSetting ...) would be neater.
This appears to be the only caller outside of HighlightStyle.
Contributor
Author
There was a problem hiding this comment.
I had a bit of a déjà vu with this class and now I know why!
DavidSpickett
left a comment
DavidSpickett
left a comment
Contributor
There was a problem hiding this comment.
Slightly torn because the AnsiPrefix naming is more clear to me what it actually is, but I do like bundling the suffix and prefix together.
I think the latter is better on balance though.
| GetTarget().GetDebugger().GetDisabledColor().GetPrefix())); | ||
|
|
||
| if (!m_kind_description.empty()) { | ||
| if (level == eDescriptionLevelBrief) { |
Contributor
There was a problem hiding this comment.
@DavidSpickett here is what I meant, expand the diff and there will be an early return
Contributor
There was a problem hiding this comment.
I see it now. If we return here the suffix is not emitted.
JDevlieghere
left a comment
JDevlieghere
left a comment
Member
There was a problem hiding this comment.
Only nits. I like Felipe's idea though.
| @@ -0,0 +1,42 @@ | |||
| //===-- ColorSetting.h ------------------------------------------*- C++ -*-===// | |||
Member
There was a problem hiding this comment.
Suggested change
| //===-- ColorSetting.h ------------------------------------------*- C++ -*-===// | |
| //===----------------------------------------------------------------------===// |
| @@ -0,0 +1,22 @@ | |||
| //===-- ColorSetting.cpp --------------------------------------------------===// | |||
Member
There was a problem hiding this comment.
Suggested change
| //===-- ColorSetting.cpp --------------------------------------------------===// | |
| //===----------------------------------------------------------------------===// |
bulbazord
reviewed
Jul 20, 2026
bulbazord
left a comment
bulbazord
left a comment
Member
There was a problem hiding this comment.
Idea looks good to me, I do like Felipe's idea a lot. Seems easy to accidentally drop a suffix via early return if we're not enforcing use of render. RAII seems like the right pattern to address that.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We use pairs of prefix/suffix strings across our code base to represent the ANSI color codes emitted before/after a highlighted word.
This patch replaces these pairs with a single ColorSettings class. The motivation is just to get rid of all the duplicated prefix/suffix variables and functions everywhere.