[SBProgress] Add swig support for with statement in Python

[Jlalond]

We recently added an explicit finalize to SBProgress, #128966. I realized while adding some additional implementations of SBProgress that we should to add with support for ease of use. This patch addresses adding and __enter()__ method (which a no-op) and an __exit()__ to swig. I also refactor the emitter for the test to leverage with instead of explicitly calling finalize, and I've updated the docstrings.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

We recently added an explicit finalize to SBProgress, #128966. I realized while adding some additional implementations of SBProgress that we should to add with support for ease of use. This patch addresses adding and __enter()__ method (which a no-op) and an __exit()__ to swig. I also refactor the emitter for the test to leverage with instead of explicitly calling finalize, and I've updated the docstrings.

---

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

4 Files Affected:

• (modified) lldb/bindings/interface/SBProgressDocstrings.i (+7)
• (added) lldb/bindings/interface/SBProgressExtensions.i (+13)
• (modified) lldb/bindings/interfaces.swig (+1)
• (modified) lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py (+10-13)

diff --git a/lldb/bindings/interface/SBProgressDocstrings.i b/lldb/bindings/interface/SBProgressDocstrings.i
index 8d252ef1f370c..0aff4c502f3a4 100644
--- a/lldb/bindings/interface/SBProgressDocstrings.i
+++ b/lldb/bindings/interface/SBProgressDocstrings.i
@@ -52,6 +52,13 @@ Non-deterministic progresses behave the same, but omit the total in the construc
     # Explicitly send a progressEnd, otherwise this will be sent
     # when the python runtime cleans up this object.
     non_deterministic_progress.Finalize()
+
+Additionally for Python, progress is supported in a with statement. ::
+    with lldb.SBProgress('Non deterministic progress, 'Detail', lldb.SBDebugger) as progress:
+        for i in range(10):
+            progress.Increment(1)
+    # The progress object is automatically finalized when the with statement
+
 ") lldb::SBProgress;    
 
 %feature("docstring",
diff --git a/lldb/bindings/interface/SBProgressExtensions.i b/lldb/bindings/interface/SBProgressExtensions.i
new file mode 100644
index 0000000000000..6ecf3a1af93b7
--- /dev/null
+++ b/lldb/bindings/interface/SBProgressExtensions.i
@@ -0,0 +1,13 @@
+%extend lldb::SBProgress {
+#ifdef SWIGPYTHON
+    %pythoncode %{
+        def __enter__(self):
+            '''No-op for with statement'''
+            pass
+
+        def __exit__(self, exc_type, exc_value, traceback):
+            '''Finalize the progress object'''
+            self.Finalize()
+    %}
+#endif
+}
\ No newline at end of file
diff --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig
index 08df9a1a8d539..6da56e4e0fa52 100644
--- a/lldb/bindings/interfaces.swig
+++ b/lldb/bindings/interfaces.swig
@@ -200,6 +200,7 @@
 %include "./interface/SBModuleSpecExtensions.i"
 %include "./interface/SBModuleSpecListExtensions.i"
 %include "./interface/SBProcessExtensions.i"
+%include "./interface/SBProgressExtensions.i"
 %include "./interface/SBProcessInfoListExtensions.i"
 %include "./interface/SBQueueItemExtensions.i"
 %include "./interface/SBScriptObjectExtensions.i"
diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
index e94a09676e067..445d1bdf4e496 100644
--- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -88,21 +88,18 @@ def __call__(self, debugger, command, exe_ctx, result):
             progress = lldb.SBProgress(
                 "Progress tester", "Initial Detail", total, debugger
             )
-
         # Check to see if total is set to None to indicate an indeterminate progress
         # then default to 10 steps.
-        if total is None:
-            total = 10
-
-        for i in range(1, total):
-            if cmd_options.no_details:
-                progress.Increment(1)
-            else:
-                progress.Increment(1, f"Step {i}")
-            time.sleep(cmd_options.seconds)
-
-        # Not required for deterministic progress, but required for indeterminate progress.
-        progress.Finalize()
+        with progress:
+            if total is None:
+                total = 10
+
+            for i in range(1, total):
+                if cmd_options.no_details:
+                    progress.Increment(1)
+                else:
+                    progress.Increment(1, f"Step {i}")
+                time.sleep(cmd_options.seconds)
 
 
 def __lldb_init_module(debugger, dict):

[Jlalond]

@JDevlieghere @bulbazord Is there a Discourse on the recent changes to the terminal colors? On Linux I'm getting this white bar that crosses the whole terminal. I think this is part of your reverse video change Jonas?

[JDevlieghere]

@JDevlieghere @bulbazord Is there a Discourse on the recent changes to the terminal colors? On Linux I'm getting this white bar that crosses the whole terminal. I think this is part of your reverse video change Jonas?

Yes, what you're seeing is the new Statusline. The reverse video change (#133315) just changes the default color. Based on your screenshot, that looks expected.

[aperez]

Just a nit, the string in the first argument is not terminated:

with lldb.SBProgress('Non deterministic progress', 'Detail', lldb.SBDebugger) as progress:

[Jlalond]

It's copied from above, and that was also unescaped, good catch

[Jlalond]

@JDevlieghere @bulbazord Is there a Discourse on the recent changes to the terminal colors? On Linux I'm getting this white bar that crosses the whole terminal. I think this is part of your reverse video change Jonas?

Yes, what you're seeing is the new Statusline. The reverse video change (#133315) just changes the default color. Based on your screenshot, that looks expected.

Neat! While I don't love the reverse video, is there an easier way to update the status line color other than setting the entire format like so?

settings set statusline-format "${ansi.bg.blue}{${target.file.basename}}{ | ${line.file.basename}:${line.number}:${line.column}}{ | ${thread.stop-reason}}{ | {${progress.count} }${progress.message}}"

If not, I'd love to contribute to make this a bit easier to update the color. Otherwise it's really neat!

[JDevlieghere]

Neat! While I don't love the reverse video, is there an easier way to update the status line color other than setting the entire format like so?

settings set statusline-format "${ansi.bg.blue}{${target.file.basename}}{ | ${line.file.basename}:${line.number}:${line.column}}{ | ${thread.stop-reason}}{ | {${progress.count} }${progress.message}}"

If not, I'd love to contribute to make this a bit easier to update the color. Otherwise it's really neat!

Yup, that's the way to do it. I agree that the reverse video doesn't look "great", but it's the only thing that is guaranteed to look "alright" with any color scheme. I played around with the colors and there's always one color scheme where a combination of colors looks horrible.

As to ways to make it easier to configure this, we generally have two ways to do this:

1. Using a prefix/suffix setting for things that don't take format strings. For example show-progress-ansi-prefix (now deprecated), show-autosuggestion-ansi-prefix, etc. The primary downside of that approach is that you have two settings.
2. Using a format strings so you can change the color of its components. For example frame-format, thread-format, etc. That's the approach I went with here as it follows the existing pattern, but it also allows you to have a different background color for things like the target, the stop reason, etc. Very similar to the vim statusline (and often folks do the same with tmux).

What I was thinking is that maybe we could extend LLDB to have the notion of "themes". That would allow you to tweak the colors without changing any of the individual settings. Plus, it would ensure that colors remain consistent: if two things were originally one color, they'd both be the new color, rather than having you to go through all the settings and finding other instances of things being that color.

[JDevlieghere]

Nice improvement!

Not sure how hard it would be to extend this even more, but Python's rich package has a progress bar that allows you to write:

for i in track(range(20), description="Processing..."):
    time.sleep(1)  # Simulate work being done

It would be cool if we could add an extension so you could write the same thing with LLDB's progress:

for i in lldb.Progress.track(range(20), description="Processing..."):
    time.sleep(1)  # Simulate work being done

[Jlalond]

Neat! While I don't love the reverse video, is there an easier way to update the status line color other than setting the entire format like so?

settings set statusline-format "${ansi.bg.blue}{${target.file.basename}}{ | ${line.file.basename}:${line.number}:${line.column}}{ | ${thread.stop-reason}}{ | {${progress.count} }${progress.message}}"

If not, I'd love to contribute to make this a bit easier to update the color. Otherwise it's really neat!

Yup, that's the way to do it. I agree that the reverse video doesn't look "great", but it's the only thing that is guaranteed to look "alright" with any color scheme. I played around with the colors and there's always one color scheme where a combination of colors looks horrible.

As to ways to make it easier to configure this, we generally have two ways to do this:

1. Using a prefix/suffix setting for things that don't take format strings. For example show-progress-ansi-prefix (now deprecated), show-autosuggestion-ansi-prefix, etc. The primary downside of that approach is that you have two settings.
2. Using a format strings so you can change the color of its components. For example frame-format, thread-format, etc. That's the approach I went with here as it follows the existing pattern, but it also allows you to have a different background color for things like the target, the stop reason, etc. Very similar to the vim statusline (and often folks do the same with tmux).

What I was thinking is that maybe we could extend LLDB to have the notion of "themes". That would allow you to tweak the colors without changing any of the individual settings. Plus, it would ensure that colors remain consistent: if two things were originally one color, they'd both be the new color, rather than having you to go through all the settings and finding other instances of things being that color.

An alternative to making it a setting it just make it exposed through the SB API as structured data, and then we can programmatically update the prefix (or expose a command to do so). I'll comment on your RFC.

[Jlalond]

Nice improvement!

Not sure how hard it would be to extend this even more, but Python's rich package has a progress bar that allows you to write:

for i in track(range(20), description="Processing..."):
    time.sleep(1)  # Simulate work being done

It would be cool if we could add an extension so you could write the same thing with LLDB's progress:

for i in lldb.Progress.track(range(20), description="Processing..."):
    time.sleep(1)  # Simulate work being done

@aperez Thoughts on this? I think this could be useful

[aperez]

@aperez Thoughts on this? I think this could be useful

I think it could be useful as well.

Couple things to note though:

• If you allow for both sequences and iterators as arguments into lldb.Progress.track you might not know the total number steps in advance.
• You'll need to handle the case when users exit the loop early, so that Finalize() is still called.