[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
https://github.com/junior-jl edited https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
=?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: llvmbot wrote: @llvm/pr-subscribers-lldb Author: José Lira Junior (junior-jl) Changes Follow-up to #69422. This commit builds upon the previous contribution that introduced symbol colorization in the image lookup command when using regex. In response to feedback from reviewers, this follow-up refines the colorization mechanism based on their recommendations. Changes: - Refactors symbol colorization logic to incorporate feedback from the previous commit. - Extends colorization to functions in addition to symbols for a more comprehensive visual representation. Co-authored-by: Talha Tahir--- Full diff: https://github.com/llvm/llvm-project/pull/76112.diff 9 Files Affected: - (modified) lldb/include/lldb/Core/Address.h (+3-1) - (modified) lldb/include/lldb/Symbol/Symbol.h (+2-1) - (modified) lldb/include/lldb/Symbol/SymbolContext.h (+3-2) - (modified) lldb/include/lldb/Utility/Stream.h (+11-3) - (modified) lldb/source/Commands/CommandObjectTarget.cpp (+24-14) - (modified) lldb/source/Core/Address.cpp (+9-9) - (modified) lldb/source/Symbol/Symbol.cpp (+3-5) - (modified) lldb/source/Symbol/SymbolContext.cpp (+7-7) - (modified) lldb/source/Utility/Stream.cpp (+4-6) ``diff diff --git a/lldb/include/lldb/Core/Address.h b/lldb/include/lldb/Core/Address.h index 725b5d9f91d3d5..c5a47321c774e7 100644 --- a/lldb/include/lldb/Core/Address.h +++ b/lldb/include/lldb/Core/Address.h @@ -13,6 +13,7 @@ #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" #include "lldb/lldb-types.h" +#include "lldb/Utility/Stream.h" #include "llvm/ADT/StringRef.h" @@ -255,7 +256,8 @@ class Address { bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style, DumpStyle fallback_style = DumpStyleInvalid, uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false, -llvm::StringRef pattern = "") const; +std::optional pattern_info = std::nullopt) const; + AddressClass GetAddressClass() const; diff --git a/lldb/include/lldb/Symbol/Symbol.h b/lldb/include/lldb/Symbol/Symbol.h index e6c0b495bcf28c..96ba7ba282a01c 100644 --- a/lldb/include/lldb/Symbol/Symbol.h +++ b/lldb/include/lldb/Symbol/Symbol.h @@ -16,6 +16,7 @@ #include "lldb/Utility/UserID.h" #include "lldb/lldb-private.h" #include "llvm/Support/JSON.h" +#include "lldb/Utility/Stream.h" namespace lldb_private { @@ -175,7 +176,7 @@ class Symbol : public SymbolContextScope { void SetFlags(uint32_t flags) { m_flags = flags; } void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - llvm::StringRef pattern = "") const; + std::optional pattern_info = std::nullopt) const; bool IsSynthetic() const { return m_is_synthetic; } diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 26f3bac09a9626..529dc9630840b7 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -17,6 +17,7 @@ #include "lldb/Core/Mangled.h" #include "lldb/Symbol/LineEntry.h" #include "lldb/Utility/Iterable.h" +#include "lldb/Utility/Stream.h" #include "lldb/lldb-private.h" namespace lldb_private { @@ -157,7 +158,7 @@ class SymbolContext { const Address &so_addr, bool show_fullpaths, bool show_module, bool show_inlined_frames, bool show_function_arguments, bool show_function_name, - llvm::StringRef pattern = "") const; + std::optional pattern_info = std::nullopt) const; /// Get the address range contained within a symbol context. /// @@ -224,7 +225,7 @@ class SymbolContext { const Symbol *FindBestGlobalDataSymbol(ConstString name, Status &error); void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - llvm::StringRef pattern = "") const; + std::optional pattern_info = std::nullopt) const; uint32_t GetResolvedMask() const; diff --git a/lldb/include/lldb/Utility/Stream.h b/lldb/include/lldb/Utility/Stream.h index 20c55ac4597ae6..7aed0a831631bd 100644 --- a/lldb/include/lldb/Utility/Stream.h +++ b/lldb/include/lldb/Utility/Stream.h @@ -23,6 +23,16 @@ namespace lldb_private { +struct Information { +llvm::StringRef pattern; +llvm::StringRef prefix; +llvm::StringRef suffix; + +// Constructor +Information(llvm::StringRef p, llvm::StringRef pre, llvm::StringRef suf) +: pattern(p), prefix(pre), suffix(suf) {} +}; + /// \class Stream Stream.h "lldb/Utility/Stream.h" /// A stream class that can stream formatted output to a file. class Stream { @@ -261,9 +271,7 @@ class Stream { /// environment-dependent. void PutCStringColorHighlighted(llvm::StringRef text, -
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
=?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 912506b75d8a508353a701c230e73ca45a253161 648ec67f79e91ad4baaec40df3f07bf60fe5f8a7 -- lldb/include/lldb/Core/Address.h lldb/include/lldb/Symbol/Symbol.h lldb/include/lldb/Symbol/SymbolContext.h lldb/include/lldb/Utility/Stream.h lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Core/Address.cpp lldb/source/Symbol/Symbol.cpp lldb/source/Symbol/SymbolContext.cpp lldb/source/Utility/Stream.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/include/lldb/Core/Address.h b/lldb/include/lldb/Core/Address.h index c5a47321c7..80c43bbe00 100644 --- a/lldb/include/lldb/Core/Address.h +++ b/lldb/include/lldb/Core/Address.h @@ -9,11 +9,11 @@ #ifndef LLDB_CORE_ADDRESS_H #define LLDB_CORE_ADDRESS_H +#include "lldb/Utility/Stream.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" #include "lldb/lldb-types.h" -#include "lldb/Utility/Stream.h" #include "llvm/ADT/StringRef.h" @@ -257,7 +257,6 @@ public: DumpStyle fallback_style = DumpStyleInvalid, uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false, std::optional pattern_info = std::nullopt) const; - AddressClass GetAddressClass() const; diff --git a/lldb/include/lldb/Symbol/Symbol.h b/lldb/include/lldb/Symbol/Symbol.h index 96ba7ba282..f5ba14be75 100644 --- a/lldb/include/lldb/Symbol/Symbol.h +++ b/lldb/include/lldb/Symbol/Symbol.h @@ -13,10 +13,10 @@ #include "lldb/Core/Mangled.h" #include "lldb/Core/Section.h" #include "lldb/Symbol/SymbolContextScope.h" +#include "lldb/Utility/Stream.h" #include "lldb/Utility/UserID.h" #include "lldb/lldb-private.h" #include "llvm/Support/JSON.h" -#include "lldb/Utility/Stream.h" namespace lldb_private { @@ -175,8 +175,9 @@ public: void SetFlags(uint32_t flags) { m_flags = flags; } - void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - std::optional pattern_info = std::nullopt) const; + void + GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, + std::optional pattern_info = std::nullopt) const; bool IsSynthetic() const { return m_is_synthetic; } diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 529dc96308..74be06e742 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -154,11 +154,12 @@ public: /// /// \return /// \b true if some text was dumped, \b false otherwise. - bool DumpStopContext(Stream *s, ExecutionContextScope *exe_scope, - const Address &so_addr, bool show_fullpaths, - bool show_module, bool show_inlined_frames, - bool show_function_arguments, bool show_function_name, - std::optional pattern_info = std::nullopt) const; + bool + DumpStopContext(Stream *s, ExecutionContextScope *exe_scope, + const Address &so_addr, bool show_fullpaths, bool show_module, + bool show_inlined_frames, bool show_function_arguments, + bool show_function_name, + std::optional pattern_info = std::nullopt) const; /// Get the address range contained within a symbol context. /// @@ -224,8 +225,9 @@ public: /// The symbol that was found, or \b nullptr if none was found. const Symbol *FindBestGlobalDataSymbol(ConstString name, Status &error); - void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - std::optional pattern_info = std::nullopt) const; + void + GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, + std::optional pattern_info = std::nullopt) const; uint32_t GetResolvedMask() const; diff --git a/lldb/include/lldb/Utility/Stream.h b/lldb/include/lldb/Utility/Stream.h index 7aed0a8316..00c9d15b19 100644 --- a/lldb/include/lldb/Utility/Stream.h +++ b/lldb/include/lldb/Utility/Stream.h @@ -24,13 +24,13 @@ namespace lldb_private { struct Information { -llvm::StringRef pattern; -llvm::StringRef prefix; -llvm::StringRef suffix; - -// Constructor -Information(llvm::StringRef p, llvm::StringRef pre, llvm::StringRef suf) -: pattern(p), prefix(pre), suffix(suf) {} + llvm::StringRef pattern; + llvm::StringRef prefix; + llvm::StringRef suffix; + + // Constructor + Information(llvm::StringRef p, llvm::StringRef pre, llvm::StringRef suf) + : pattern(p), prefix(pre), suffix(suf) {} }; /// \class Stream Stream.h "lldb/Utility/Stream.h" @@ -270,8 +270,9 @@ public: //
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
https://github.com/junior-jl updated https://github.com/llvm/llvm-project/pull/76112 From b2d254fabcaacb849ba48e342c62abf7ecd9779a Mon Sep 17 00:00:00 2001 From: taalhaataahir0102 <23100...@lums.edu.pk> Date: Wed, 13 Dec 2023 15:12:29 +0500 Subject: [PATCH 1/5] Using struct for transfering pattern information --- lldb/include/lldb/Core/Address.h | 4 +- lldb/include/lldb/Symbol/Symbol.h| 3 +- lldb/include/lldb/Symbol/SymbolContext.h | 5 +- lldb/include/lldb/Utility/Stream.h | 10 +++- lldb/source/Commands/CommandObjectTarget.cpp | 35 lldb/source/Core/Address.cpp | 59 ++-- lldb/source/Symbol/Symbol.cpp| 14 +++-- lldb/source/Symbol/SymbolContext.cpp | 22 +--- lldb/source/Utility/Stream.cpp | 10 ++-- 9 files changed, 110 insertions(+), 52 deletions(-) diff --git a/lldb/include/lldb/Core/Address.h b/lldb/include/lldb/Core/Address.h index 725b5d9f91d3d5..c5a47321c774e7 100644 --- a/lldb/include/lldb/Core/Address.h +++ b/lldb/include/lldb/Core/Address.h @@ -13,6 +13,7 @@ #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" #include "lldb/lldb-types.h" +#include "lldb/Utility/Stream.h" #include "llvm/ADT/StringRef.h" @@ -255,7 +256,8 @@ class Address { bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style, DumpStyle fallback_style = DumpStyleInvalid, uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false, -llvm::StringRef pattern = "") const; +std::optional pattern_info = std::nullopt) const; + AddressClass GetAddressClass() const; diff --git a/lldb/include/lldb/Symbol/Symbol.h b/lldb/include/lldb/Symbol/Symbol.h index e6c0b495bcf28c..96ba7ba282a01c 100644 --- a/lldb/include/lldb/Symbol/Symbol.h +++ b/lldb/include/lldb/Symbol/Symbol.h @@ -16,6 +16,7 @@ #include "lldb/Utility/UserID.h" #include "lldb/lldb-private.h" #include "llvm/Support/JSON.h" +#include "lldb/Utility/Stream.h" namespace lldb_private { @@ -175,7 +176,7 @@ class Symbol : public SymbolContextScope { void SetFlags(uint32_t flags) { m_flags = flags; } void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - llvm::StringRef pattern = "") const; + std::optional pattern_info = std::nullopt) const; bool IsSynthetic() const { return m_is_synthetic; } diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 26f3bac09a9626..529dc9630840b7 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -17,6 +17,7 @@ #include "lldb/Core/Mangled.h" #include "lldb/Symbol/LineEntry.h" #include "lldb/Utility/Iterable.h" +#include "lldb/Utility/Stream.h" #include "lldb/lldb-private.h" namespace lldb_private { @@ -157,7 +158,7 @@ class SymbolContext { const Address &so_addr, bool show_fullpaths, bool show_module, bool show_inlined_frames, bool show_function_arguments, bool show_function_name, - llvm::StringRef pattern = "") const; + std::optional pattern_info = std::nullopt) const; /// Get the address range contained within a symbol context. /// @@ -224,7 +225,7 @@ class SymbolContext { const Symbol *FindBestGlobalDataSymbol(ConstString name, Status &error); void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - llvm::StringRef pattern = "") const; + std::optional pattern_info = std::nullopt) const; uint32_t GetResolvedMask() const; diff --git a/lldb/include/lldb/Utility/Stream.h b/lldb/include/lldb/Utility/Stream.h index 20c55ac4597ae6..accbdb3b3fed35 100644 --- a/lldb/include/lldb/Utility/Stream.h +++ b/lldb/include/lldb/Utility/Stream.h @@ -23,6 +23,12 @@ namespace lldb_private { +struct Information { +llvm::StringRef pattern; +llvm::StringRef prefix; +llvm::StringRef suffix; +}; + /// \class Stream Stream.h "lldb/Utility/Stream.h" /// A stream class that can stream formatted output to a file. class Stream { @@ -261,9 +267,7 @@ class Stream { /// environment-dependent. void PutCStringColorHighlighted(llvm::StringRef text, - llvm::StringRef pattern = "", - llvm::StringRef prefix = "", - llvm::StringRef suffix = ""); + std::optional pattern_info = std::nullopt); /// Output and End of Line character to the stream. size_t EOL(); diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index bc8bc51356c8ca..96c7b2f5930cd1 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Comma
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
https://github.com/junior-jl updated https://github.com/llvm/llvm-project/pull/76112 From b2d254fabcaacb849ba48e342c62abf7ecd9779a Mon Sep 17 00:00:00 2001 From: taalhaataahir0102 <23100...@lums.edu.pk> Date: Wed, 13 Dec 2023 15:12:29 +0500 Subject: [PATCH 1/6] Using struct for transfering pattern information --- lldb/include/lldb/Core/Address.h | 4 +- lldb/include/lldb/Symbol/Symbol.h| 3 +- lldb/include/lldb/Symbol/SymbolContext.h | 5 +- lldb/include/lldb/Utility/Stream.h | 10 +++- lldb/source/Commands/CommandObjectTarget.cpp | 35 lldb/source/Core/Address.cpp | 59 ++-- lldb/source/Symbol/Symbol.cpp| 14 +++-- lldb/source/Symbol/SymbolContext.cpp | 22 +--- lldb/source/Utility/Stream.cpp | 10 ++-- 9 files changed, 110 insertions(+), 52 deletions(-) diff --git a/lldb/include/lldb/Core/Address.h b/lldb/include/lldb/Core/Address.h index 725b5d9f91d3d5..c5a47321c774e7 100644 --- a/lldb/include/lldb/Core/Address.h +++ b/lldb/include/lldb/Core/Address.h @@ -13,6 +13,7 @@ #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" #include "lldb/lldb-types.h" +#include "lldb/Utility/Stream.h" #include "llvm/ADT/StringRef.h" @@ -255,7 +256,8 @@ class Address { bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style, DumpStyle fallback_style = DumpStyleInvalid, uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false, -llvm::StringRef pattern = "") const; +std::optional pattern_info = std::nullopt) const; + AddressClass GetAddressClass() const; diff --git a/lldb/include/lldb/Symbol/Symbol.h b/lldb/include/lldb/Symbol/Symbol.h index e6c0b495bcf28c..96ba7ba282a01c 100644 --- a/lldb/include/lldb/Symbol/Symbol.h +++ b/lldb/include/lldb/Symbol/Symbol.h @@ -16,6 +16,7 @@ #include "lldb/Utility/UserID.h" #include "lldb/lldb-private.h" #include "llvm/Support/JSON.h" +#include "lldb/Utility/Stream.h" namespace lldb_private { @@ -175,7 +176,7 @@ class Symbol : public SymbolContextScope { void SetFlags(uint32_t flags) { m_flags = flags; } void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - llvm::StringRef pattern = "") const; + std::optional pattern_info = std::nullopt) const; bool IsSynthetic() const { return m_is_synthetic; } diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 26f3bac09a9626..529dc9630840b7 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -17,6 +17,7 @@ #include "lldb/Core/Mangled.h" #include "lldb/Symbol/LineEntry.h" #include "lldb/Utility/Iterable.h" +#include "lldb/Utility/Stream.h" #include "lldb/lldb-private.h" namespace lldb_private { @@ -157,7 +158,7 @@ class SymbolContext { const Address &so_addr, bool show_fullpaths, bool show_module, bool show_inlined_frames, bool show_function_arguments, bool show_function_name, - llvm::StringRef pattern = "") const; + std::optional pattern_info = std::nullopt) const; /// Get the address range contained within a symbol context. /// @@ -224,7 +225,7 @@ class SymbolContext { const Symbol *FindBestGlobalDataSymbol(ConstString name, Status &error); void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - llvm::StringRef pattern = "") const; + std::optional pattern_info = std::nullopt) const; uint32_t GetResolvedMask() const; diff --git a/lldb/include/lldb/Utility/Stream.h b/lldb/include/lldb/Utility/Stream.h index 20c55ac4597ae6..accbdb3b3fed35 100644 --- a/lldb/include/lldb/Utility/Stream.h +++ b/lldb/include/lldb/Utility/Stream.h @@ -23,6 +23,12 @@ namespace lldb_private { +struct Information { +llvm::StringRef pattern; +llvm::StringRef prefix; +llvm::StringRef suffix; +}; + /// \class Stream Stream.h "lldb/Utility/Stream.h" /// A stream class that can stream formatted output to a file. class Stream { @@ -261,9 +267,7 @@ class Stream { /// environment-dependent. void PutCStringColorHighlighted(llvm::StringRef text, - llvm::StringRef pattern = "", - llvm::StringRef prefix = "", - llvm::StringRef suffix = ""); + std::optional pattern_info = std::nullopt); /// Output and End of Line character to the stream. size_t EOL(); diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index bc8bc51356c8ca..96c7b2f5930cd1 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Comma
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
=?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: taalhaataahir0102 wrote: Hi! @DavidSpickett, made the above changes. 1. struct has been introduced inside stream.h and all the information is being passed using optional struct (`std::optional pattern_info`) and all the member variables inside the struct are `llvm::StringRef`. Were not able to solve the issue for `llvm::regex` 2. For function search, it was similar to symbol search as it also passes all the information to the same dumping functions. 3. For test cases, Right now all the test cases are for -s flag (symbol search) i..e, 11 in total. Should we split the current test cases like 4 for symbol search, 4 for function search (-F flag) and 3 for symbol-or-function search (-n flag) or we should add more test cases for each flag? https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
=?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: @@ -72,23 +72,21 @@ size_t Stream::PutCString(llvm::StringRef str) { return bytes_written; } -void Stream::PutCStringColorHighlighted(llvm::StringRef text, -llvm::StringRef pattern, -llvm::StringRef prefix, -llvm::StringRef suffix) { - // Only apply color formatting when a pattern is present and both prefix and - // suffix are specified. In the absence of these conditions, output the text - // without color formatting. - if (pattern.empty() || (prefix.empty() && suffix.empty())) { +void Stream::PutCStringColorHighlighted( +llvm::StringRef text, std::optional pattern_info) { + // Only apply color formatting when the pattern information is specified. + // Otherwise, output the text without color formatting. + if (!pattern_info.has_value()) { PutCString(text); return; } - llvm::Regex reg_pattern(pattern); + llvm::Regex reg_pattern(pattern_info.value().pattern); DavidSpickett wrote: This isn't wrong but you can instead do: ``` pattern_info->pattern ``` Pointer like operations on optionals accessed the contained value. https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
=?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: @@ -262,14 +263,12 @@ void Symbol::GetDescription(Stream *s, lldb::DescriptionLevel level, } if (ConstString demangled = m_mangled.GetDemangledName()) { s->PutCString(", name=\""); -s->PutCStringColorHighlighted(demangled.GetStringRef(), pattern, - ansi_prefix, ansi_suffix); +s->PutCStringColorHighlighted(demangled.GetStringRef(), pattern_info); s->PutCString("\""); } if (ConstString mangled_name = m_mangled.GetMangledName()) { s->PutCString(", mangled=\""); -s->PutCStringColorHighlighted(mangled_name.GetStringRef(), pattern, - ansi_prefix, ansi_suffix); +s->PutCStringColorHighlighted(mangled_name.GetStringRef(), pattern_info); DavidSpickett wrote: The name `pattern_info` for a local variable is fine I think, the name of the type bothers me more. https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
=?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: @@ -1609,6 +1612,11 @@ static uint32_t LookupSymbolInModule(CommandInterpreter &interpreter, } if (num_matches > 0) { +llvm::StringRef ansi_prefix = +interpreter.GetDebugger().GetRegexMatchAnsiPrefix(); +llvm::StringRef ansi_suffix = +interpreter.GetDebugger().GetRegexMatchAnsiSuffix(); +Information info(name, ansi_prefix, ansi_suffix); DavidSpickett wrote: Construct this closer to the place of use, also inline the `GetRegex...` calls into it like `info(...GetRegexMatch...())`. https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
=?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: @@ -23,6 +23,16 @@ namespace lldb_private { +struct Information { DavidSpickett wrote: This should be within the Stream class. Also the name is too generic, `HighlightSettings` maybe? Choosing a name is hard but something that says what the information is about. https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
=?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: @@ -72,23 +72,21 @@ size_t Stream::PutCString(llvm::StringRef str) { return bytes_written; } -void Stream::PutCStringColorHighlighted(llvm::StringRef text, -llvm::StringRef pattern, -llvm::StringRef prefix, -llvm::StringRef suffix) { - // Only apply color formatting when a pattern is present and both prefix and - // suffix are specified. In the absence of these conditions, output the text - // without color formatting. - if (pattern.empty() || (prefix.empty() && suffix.empty())) { +void Stream::PutCStringColorHighlighted( +llvm::StringRef text, std::optional pattern_info) { + // Only apply color formatting when the pattern information is specified. + // Otherwise, output the text without color formatting. + if (!pattern_info.has_value()) { PutCString(text); return; } - llvm::Regex reg_pattern(pattern); + llvm::Regex reg_pattern(pattern_info.value().pattern); DavidSpickett wrote: Which can be cleaner, is my point, `value()` vs. `->`. https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
=?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior , =?utf-8?q?José?= L. Junior Message-ID: In-Reply-To: DavidSpickett wrote: Drive by comments since I am low on time, also I will be away until early January from today. A general comment, please keep each PR to one thing. So I'd say making the highlight stuff into a struct is one PR, then adding function highlighting is another. Makes it easier to understand for everyone involved. On tests for function highlighting, assuming we know that `PutCStringHighlighted` works (which we do), we don't need to repeat single match, multiple match, end of string match, etc. for functions as well. Just check that the parts of the output for functions have some sort of highlight in them. Use the simplest query that will achieve that. https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
https://github.com/junior-jl updated https://github.com/llvm/llvm-project/pull/76112 From fb2383f3e6e2124e4f14e8e0f6a04df4bed15f65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20L=2E=20Junior?= Date: Thu, 18 Jan 2024 20:03:25 -0300 Subject: [PATCH] refactor PutCStringColorHighlight | add struct to store highlight settings --- lldb/include/lldb/Core/Address.h | 4 +- lldb/include/lldb/Symbol/Symbol.h| 4 +- lldb/include/lldb/Symbol/SymbolContext.h | 7 +++- lldb/include/lldb/Utility/Stream.h | 18 +++-- lldb/source/Commands/CommandObjectTarget.cpp | 39 lldb/source/Core/Address.cpp | 22 +-- lldb/source/Symbol/Symbol.cpp| 11 +++--- lldb/source/Symbol/SymbolContext.cpp | 36 +++--- lldb/source/Utility/Stream.cpp | 17 - 9 files changed, 85 insertions(+), 73 deletions(-) diff --git a/lldb/include/lldb/Core/Address.h b/lldb/include/lldb/Core/Address.h index 725b5d9f91d3d5..f11ece414eec83 100644 --- a/lldb/include/lldb/Core/Address.h +++ b/lldb/include/lldb/Core/Address.h @@ -9,6 +9,7 @@ #ifndef LLDB_CORE_ADDRESS_H #define LLDB_CORE_ADDRESS_H +#include "lldb/Utility/Stream.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" @@ -255,7 +256,8 @@ class Address { bool Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style, DumpStyle fallback_style = DumpStyleInvalid, uint32_t addr_byte_size = UINT32_MAX, bool all_ranges = false, -llvm::StringRef pattern = "") const; +std::optional pattern_info = +std::nullopt) const; AddressClass GetAddressClass() const; diff --git a/lldb/include/lldb/Symbol/Symbol.h b/lldb/include/lldb/Symbol/Symbol.h index e6c0b495bcf28c..0da431f5ccf5da 100644 --- a/lldb/include/lldb/Symbol/Symbol.h +++ b/lldb/include/lldb/Symbol/Symbol.h @@ -13,6 +13,7 @@ #include "lldb/Core/Mangled.h" #include "lldb/Core/Section.h" #include "lldb/Symbol/SymbolContextScope.h" +#include "lldb/Utility/Stream.h" #include "lldb/Utility/UserID.h" #include "lldb/lldb-private.h" #include "llvm/Support/JSON.h" @@ -175,7 +176,8 @@ class Symbol : public SymbolContextScope { void SetFlags(uint32_t flags) { m_flags = flags; } void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - llvm::StringRef pattern = "") const; + std::optional pattern_info = + std::nullopt) const; bool IsSynthetic() const { return m_is_synthetic; } diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h index 26f3bac09a9626..a089e93863a75d 100644 --- a/lldb/include/lldb/Symbol/SymbolContext.h +++ b/lldb/include/lldb/Symbol/SymbolContext.h @@ -17,6 +17,7 @@ #include "lldb/Core/Mangled.h" #include "lldb/Symbol/LineEntry.h" #include "lldb/Utility/Iterable.h" +#include "lldb/Utility/Stream.h" #include "lldb/lldb-private.h" namespace lldb_private { @@ -157,7 +158,8 @@ class SymbolContext { const Address &so_addr, bool show_fullpaths, bool show_module, bool show_inlined_frames, bool show_function_arguments, bool show_function_name, - llvm::StringRef pattern = "") const; + std::optional pattern_info = + std::nullopt) const; /// Get the address range contained within a symbol context. /// @@ -224,7 +226,8 @@ class SymbolContext { const Symbol *FindBestGlobalDataSymbol(ConstString name, Status &error); void GetDescription(Stream *s, lldb::DescriptionLevel level, Target *target, - llvm::StringRef pattern = "") const; + std::optional pattern_info = + std::nullopt) const; uint32_t GetResolvedMask() const; diff --git a/lldb/include/lldb/Utility/Stream.h b/lldb/include/lldb/Utility/Stream.h index 20c55ac4597ae6..f2666a749d5fa1 100644 --- a/lldb/include/lldb/Utility/Stream.h +++ b/lldb/include/lldb/Utility/Stream.h @@ -33,6 +33,17 @@ class Stream { /// string mode. }; + /// 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. + +HighlightSettings(llvm::StringRef p, llvm::StringRef pre, + llvm::StringRef suf) +: pattern(p), prefix(pre), suffix(suf) {} + }; + /// Utility class for counting the bytes that were written to a stream in a /// certain time span. /// @@ -260,10 +271,9 @@ class Stream { /// The ANSI color code to end colorization. This
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
@@ -23,6 +23,16 @@ namespace lldb_private { +struct Information { junior-jl wrote: Done. I was thinking: should we add a default constructor? https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
@@ -1609,6 +1612,11 @@ static uint32_t LookupSymbolInModule(CommandInterpreter &interpreter, } if (num_matches > 0) { +llvm::StringRef ansi_prefix = +interpreter.GetDebugger().GetRegexMatchAnsiPrefix(); +llvm::StringRef ansi_suffix = +interpreter.GetDebugger().GetRegexMatchAnsiSuffix(); +Information info(name, ansi_prefix, ansi_suffix); junior-jl wrote: I'm unsure if the modification made in last commit addresses this correctly. https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
@@ -262,14 +263,12 @@ void Symbol::GetDescription(Stream *s, lldb::DescriptionLevel level, } if (ConstString demangled = m_mangled.GetDemangledName()) { s->PutCString(", name=\""); -s->PutCStringColorHighlighted(demangled.GetStringRef(), pattern, - ansi_prefix, ansi_suffix); +s->PutCStringColorHighlighted(demangled.GetStringRef(), pattern_info); s->PutCString("\""); } if (ConstString mangled_name = m_mangled.GetMangledName()) { s->PutCString(", mangled=\""); -s->PutCStringColorHighlighted(mangled_name.GetStringRef(), pattern, - ansi_prefix, ansi_suffix); +s->PutCStringColorHighlighted(mangled_name.GetStringRef(), pattern_info); junior-jl wrote: Since the struct name changed, should we rename this to something like `highlight_settings` or `color_settings`? https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
@@ -72,23 +72,21 @@ size_t Stream::PutCString(llvm::StringRef str) { return bytes_written; } -void Stream::PutCStringColorHighlighted(llvm::StringRef text, -llvm::StringRef pattern, -llvm::StringRef prefix, -llvm::StringRef suffix) { - // Only apply color formatting when a pattern is present and both prefix and - // suffix are specified. In the absence of these conditions, output the text - // without color formatting. - if (pattern.empty() || (prefix.empty() && suffix.empty())) { +void Stream::PutCStringColorHighlighted( +llvm::StringRef text, std::optional pattern_info) { + // Only apply color formatting when the pattern information is specified. + // Otherwise, output the text without color formatting. + if (!pattern_info.has_value()) { PutCString(text); return; } - llvm::Regex reg_pattern(pattern); + llvm::Regex reg_pattern(pattern_info.value().pattern); junior-jl wrote: Done. Thanks! https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)
https://github.com/junior-jl edited https://github.com/llvm/llvm-project/pull/76112 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits