[Lldb-commits] [lldb] [lldb] enhance colorize process for image lookup command (PR #76112)

[José Lira Junior via lldb-commits]

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)

[via lldb-commits]

=?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)

[via lldb-commits]

=?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)

[José Lira Junior via lldb-commits]

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)

[José Lira Junior via lldb-commits]

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)

[via lldb-commits]

=?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)

[David Spickett via lldb-commits]

=?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)

[David Spickett via lldb-commits]

=?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)

[David Spickett via lldb-commits]

=?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)

[David Spickett via lldb-commits]

=?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)

[David Spickett via lldb-commits]

=?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)

[David Spickett via lldb-commits]

=?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)

[José Lira Junior via lldb-commits]

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)

[José Lira Junior via lldb-commits]

@@ -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)

[José Lira Junior via lldb-commits]

@@ -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)

[José Lira Junior via lldb-commits]

@@ -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)

[José Lira Junior via lldb-commits]

@@ -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)

[José Lira Junior via lldb-commits]

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