[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-07-11 Thread Chelsea Cassanova via lldb-commits 

chelcassanova wrote:

Just making sure, a setting for changing the prefix/suffix would be in 
`CoreProperties.td` right?

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-07-11 Thread Jonas Devlieghere via lldb-commits 

https://github.com/JDevlieghere commented:

This is looking good, but needs a (pexpect) test. I would also like to add a 
setting to make the prefix and suffix configurable, like we do for things like 
`show-progress-ansi-prefix` and `show-autosuggestion-ansi-prefix`. That way 
user can pick their own preferred color, or even just turn this one feature off 
without having to disable color if they want to. 

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-06-18 Thread Chelsea Cassanova via lldb-commits 

https://github.com/chelcassanova updated 
https://github.com/llvm/llvm-project/pull/91404

>From 5724d6c77d29ad80e9ca03ce7ac1c3e6ed33afc0 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova 
Date: Thu, 9 May 2024 11:08:29 -0700
Subject: [PATCH] [lldb][breakpoint] Grey out disabled breakpoints

This commit adds colour settings to the list of breakpoints in order to
grey out breakpoints that have been disabled.
---
 lldb/include/lldb/API/SBStream.h  |  5 +
 lldb/include/lldb/Utility/Stream.h|  8 
 lldb/source/API/SBStream.cpp  | 10 ++
 lldb/source/Breakpoint/Breakpoint.cpp |  9 +
 lldb/source/Utility/Stream.cpp|  8 
 5 files changed, 40 insertions(+)

diff --git a/lldb/include/lldb/API/SBStream.h b/lldb/include/lldb/API/SBStream.h
index 0e33f05b69916..108ddc38b4028 100644
--- a/lldb/include/lldb/API/SBStream.h
+++ b/lldb/include/lldb/API/SBStream.h
@@ -12,6 +12,7 @@
 #include 
 
 #include "lldb/API/SBDefines.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace lldb {
 
@@ -43,6 +44,10 @@ class LLDB_API SBStream {
 
   void Print(const char *str);
 
+  bool HasColor();
+
+  void FormatAnsiTerminalCodes(llvm::StringRef format);
+
   void RedirectToFile(const char *path, bool append);
 
   void RedirectToFile(lldb::SBFile file);
diff --git a/lldb/include/lldb/Utility/Stream.h 
b/lldb/include/lldb/Utility/Stream.h
index 37bcdc9924171..1ab590202cd69 100644
--- a/lldb/include/lldb/Utility/Stream.h
+++ b/lldb/include/lldb/Utility/Stream.h
@@ -309,6 +309,12 @@ class Stream {
   /// The current indentation level.
   unsigned GetIndentLevel() const;
 
+  /// Whether or not the stream is using color.
+  ///
+  /// \return
+  /// The color setting of the stream.
+  bool HasColor();
+
   /// Indent the current line in the stream.
   ///
   /// Indent the current line using the current indentation level and print an
@@ -366,6 +372,8 @@ class Stream {
   /// The optional C string format that can be overridden.
   void QuotedCString(const char *cstr, const char *format = "\"%s\"");
 
+  void FormatAnsiTerminalCodes(llvm::StringRef format);
+
   /// Set the address size in bytes.
   ///
   /// \param[in] addr_size
diff --git a/lldb/source/API/SBStream.cpp b/lldb/source/API/SBStream.cpp
index fc8f09a7bb9ae..bc0f3356d4753 100644
--- a/lldb/source/API/SBStream.cpp
+++ b/lldb/source/API/SBStream.cpp
@@ -11,6 +11,7 @@
 #include "lldb/API/SBFile.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/StreamFile.h"
+#include "lldb/Utility/AnsiTerminal.h"
 #include "lldb/Utility/Instrumentation.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Status.h"
@@ -77,6 +78,15 @@ void SBStream::Printf(const char *format, ...) {
   va_end(args);
 }
 
+bool SBStream::HasColor() {
+  return m_opaque_up->AsRawOstream().colors_enabled();
+}
+
+void SBStream::FormatAnsiTerminalCodes(llvm::StringRef format) {
+  if (HasColor())
+Printf("%s", ansi::FormatAnsiTerminalCodes(format).c_str());
+}
+
 void SBStream::RedirectToFile(const char *path, bool append) {
   LLDB_INSTRUMENT_VA(this, path, append);
 
diff --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index ae845e92762b9..95624f4ae3ad5 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Breakpoint/BreakpointResolver.h"
 #include "lldb/Breakpoint/BreakpointResolverFileLine.h"
 #include "lldb/Core/Address.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/SearchFilter.h"
@@ -26,6 +27,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadSpec.h"
+#include "lldb/Utility/AnsiTerminal.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
@@ -837,6 +839,10 @@ void Breakpoint::GetDescription(Stream *s, 
lldb::DescriptionLevel level,
 bool show_locations) {
   assert(s != nullptr);
 
+  // Grey out any disabled breakpoints in the list of breakpoints.
+  if (!IsEnabled())
+s->FormatAnsiTerminalCodes("${ansi.faint}");
+
   if (!m_kind_description.empty()) {
 if (level == eDescriptionLevelBrief) {
   s->PutCString(GetBreakpointKind());
@@ -933,6 +939,9 @@ void Breakpoint::GetDescription(Stream *s, 
lldb::DescriptionLevel level,
 }
 s->IndentLess();
   }
+
+  // Reset the colors back to normal if they were previously greyed out.
+  s->FormatAnsiTerminalCodes("${ansi.normal}");
 }
 
 void Breakpoint::GetResolverDescription(Stream *s) {
diff --git a/lldb/source/Utility/Stream.cpp b/lldb/source/Utility/Stream.cpp
index 89dce9fb0e1f7..e4ca9ad5a1f14 100644
--- a/lldb/source/Utility/Stream.cpp
+++ b/lldb/source/Utility/Stream.cpp
@@ -103,6 +103,11 @@ void Stream::QuotedCString(const char *cstr, const char 
*format) {
   Printf(format, cstr);
 }
 
+void

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-16 Thread Jonas Devlieghere via lldb-commits 

JDevlieghere wrote:

The stream already knows whether color support is enabled, but it looks like 
we're only storing that in the underlying llvm stream, which is commonly used 
in combination with `WithColor`. I think we could add a member to check if the 
stream has colors enabled. It's already a property of `m_forwarder` so we don't 
need to create a new member. 

```
SBStream::HasColor() {
  return m_forwarded.has_color();
}
```

Alternatively, we could add a helper that does that under the hood. 

```
SBStream::FormatAnsiTerminalCodes(llvm::StringRef format) {
  if (m_forwarded.has_color()) 
print(FormatAnsiTerminalCodes(format));
}
```

So instead of having to write:

```
if(s->HasColor())
 s->Printf("%s", ansi::FormatAnsiTerminalCodes("${ansi.faint}").c_str());
```

You can just write:

```
s-> FormatAnsiTerminalCodes("${ansi.faint}") 
```

and have it do the right thing.
```

I think I understand Jim's concern. Most places that we use colors, we do that 
through `WithColor` which checks the color property of the underlying LLVM 
stream. Chelsea is not going through WithColor, so in her case she would indeed 
need to check 

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-16 Thread Chelsea Cassanova via lldb-commits 

chelcassanova wrote:

Cool, sorry for the delay but I made changes to `Stream` and `SBStream` to add 
a `GetUseColor()` accessor and to change the bool it uses in initialization to 
a `std::optional` and I'm gonna put them up in a separate PR. 

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-10 Thread via lldb-commits 

jimingham wrote:

> Hm, so in that case should we focus on adding an `SBStream::GetUseColor` so 
> that the stream's colour settings can take precedence here?

That's the only way it makes sense to me.  But if we are going to rely on the 
stream, we also have to be sure that we're setting the Streams "use color" 
correctly when we make them.

That might be an annoying accounting exercise.

I think an easier way allow streams to both get their "use color" from context 
and also override it for special purposes is to replace the use color bool with 
a "use color, don't use color, no opinion" enum.  We'd make streams creation 
default to "no opinion".  The SB API's would instead create them with "no use 
color" - and we should add an accessor to the SB API's.  And then internally, 
"no opinion" means "consult the debugger" - but more generally it means 
"determine my use color value from context".

That saves you from having to track down all the cases where streams were made 
and get their "use color" right. 

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-10 Thread Chelsea Cassanova via lldb-commits 

chelcassanova wrote:

Hm, so in that case should we focus on adding an `SBStream::GetUseColor` so 
that the stream's colour settings can take precedence here?

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-10 Thread via lldb-commits 

jimingham wrote:

I'm not sure checking the debugger here resolves the difficulty I pointed out 
earlier.  If I make an SBStream and call SBBreakpoint::GetDescription, I will 
have passed in an SBStream with `use_color` explicitly off.  It still seems to 
me that should take precedence over the debugger's global setting.  I wanted 
this for programmatic uses presumably, and shouldn't have to deal with color 
codes...  Having to temporarily unset the global use color in the debugger to 
get this effect is not a good design.

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-09 Thread Chelsea Cassanova via lldb-commits 

https://github.com/chelcassanova updated 
https://github.com/llvm/llvm-project/pull/91404

>From 0e45adeac968aa435f58dfef026ef308e56b40a5 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova 
Date: Thu, 9 May 2024 11:08:29 -0700
Subject: [PATCH] [lldb][breakpoint] Grey out disabled breakpoints

This commit adds colour settings to the list of breakpoints in order to
grey out breakpoints that have been disabled.
---
 lldb/source/Breakpoint/Breakpoint.cpp | 12 
 1 file changed, 12 insertions(+)

diff --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index ae845e92762b9..8d28c5f1873e1 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Breakpoint/BreakpointResolver.h"
 #include "lldb/Breakpoint/BreakpointResolverFileLine.h"
 #include "lldb/Core/Address.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/SearchFilter.h"
@@ -26,6 +27,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadSpec.h"
+#include "lldb/Utility/AnsiTerminal.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
@@ -837,6 +839,12 @@ void Breakpoint::GetDescription(Stream *s, 
lldb::DescriptionLevel level,
 bool show_locations) {
   assert(s != nullptr);
 
+  // Grey out any disabled breakpoints in the list of breakpoints.
+  const bool print_faint =
+  (!IsEnabled() && GetTarget().GetDebugger().GetUseColor());
+  if (print_faint)
+s->Printf("%s", ansi::FormatAnsiTerminalCodes("${ansi.faint}").c_str());
+
   if (!m_kind_description.empty()) {
 if (level == eDescriptionLevelBrief) {
   s->PutCString(GetBreakpointKind());
@@ -933,6 +941,10 @@ void Breakpoint::GetDescription(Stream *s, 
lldb::DescriptionLevel level,
 }
 s->IndentLess();
   }
+
+  // Reset the colors back to normal if they were previously greyed out.
+  if (print_faint)
+s->Printf("%s", ansi::FormatAnsiTerminalCodes("${ansi.normal}").c_str());
 }
 
 void Breakpoint::GetResolverDescription(Stream *s) {

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-09 Thread Chelsea Cassanova via lldb-commits 


@@ -848,6 +850,13 @@ void Breakpoint::GetDescription(Stream *s, 
lldb::DescriptionLevel level,
   const size_t num_locations = GetNumLocations();
   const size_t num_resolved_locations = GetNumResolvedLocations();
 
+  // Grey out any disabled breakpoints in the list of breakpoints.
+  if (GetTarget().GetDebugger().GetUseColor())
+s->Printf("%s",
+  IsEnabled()
+  ? ansi::FormatAnsiTerminalCodes("${ansi.normal}").c_str()
+  : ansi::FormatAnsiTerminalCodes("${ansi.faint}").c_str());

chelcassanova wrote:

![image](https://github.com/llvm/llvm-project/assets/21184907/0a95ccc6-bf9a-4acb-b8e0-2746d5911f15)
I also thought that I'd have to reset the colours back to normal somewhere but 
when I tried this for a little the colours seemed fine without explicitly 
placing a reset somewhere.

I can still add a reset alongside this simplified logic though.

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-09 Thread Jonas Devlieghere via lldb-commits 


@@ -848,6 +850,13 @@ void Breakpoint::GetDescription(Stream *s, 
lldb::DescriptionLevel level,
   const size_t num_locations = GetNumLocations();
   const size_t num_resolved_locations = GetNumResolvedLocations();
 
+  // Grey out any disabled breakpoints in the list of breakpoints.
+  if (GetTarget().GetDebugger().GetUseColor())
+s->Printf("%s",
+  IsEnabled()
+  ? ansi::FormatAnsiTerminalCodes("${ansi.normal}").c_str()
+  : ansi::FormatAnsiTerminalCodes("${ansi.faint}").c_str());

JDevlieghere wrote:

You need to reset the color after printing, otherwise everything in the 
terminal printed after will be faint.  Also seems like you can simplify this by 
moving the check for `IsEnabled()` into the first `if`:

```
const bool print_faint = !IsEnabled() && 
GetTarget().GetDebugger().GetUseColor();
if (print_faint)
 s->Print(ansi::FormatAnsiTerminalCodes("${ansi.faint}"));

[...]

if (print_faint)
 s->Print(ansi::FormatAnsiTerminalCodes("${ansi.reset}"));
```


https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-09 Thread Chelsea Cassanova via lldb-commits 

https://github.com/chelcassanova edited 
https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

2024-05-09 Thread Chelsea Cassanova via lldb-commits 

https://github.com/chelcassanova edited 
https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits