Add auto-refresh capabilities for memory inspector windows #115

martin-fleck-at · 2024-03-22T15:37:35Z

Extend Auto Refresh capabilities with 'After Delay' and 'On Focus'

Rework 'refreshOnStop' boolean setting to 'autoRefresh' enumerable
-- On Stop (previously: 'on' for 'refreshOnStop')
-- On Focus (previously: always implicit on view state change)
-- After Delay (new)
-- Off (previously: 'off' for 'refreshOnStop')
On Stop
-- Rework global setting to be local for each Memory Inspector
-- Listen to debug session stopped event and propagate to view
On Focus
-- Rework implicit refresh update to option in setting
-- Listen to view state changes and propagate to view
After Delay
-- New option to explicitly define a delay when re-fetching the memory
-- Minimum: 500ms, default: 500, input step size: 250ms

Refactoring:

Split debug session tracking into dedicated class with session events
-- Convert debug events into session events with additional data
Split context tracking from memory provider into dedicated class
Move manifest to common for default values and avoid duplication
-- Align 'Min' with 'Minimal' value from manifest

Minor:

Add title toolbar item for C/C++ file to access open memory inspector
Improve debugging experience by using inline source maps
Align creation of option enums to use const objects
Additionally guard 'body' on debug responses for safety
Avoid functional React state update where unnecessary

Fixes #91

How to test

Enable auto-refresh in the display options
If your DAP provides logging you can watch the request/response pairs in the output

(Note: Options have been replaced by enum and look a little bit different now)

Review checklist

as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

as a reviewer, I agree to behave in accordance with the review guidelines

colin-grant-work · 2024-03-22T15:51:26Z

We already have the preference "memory-inspector.refreshOnStop", with which this new preference clashes a bit. I'd suggest that we generalize to something like the autosave preference with various triggers:

So that we could have, for now off onStop afterDelay + the delay preference.

martin-fleck-at · 2024-03-22T16:11:25Z

We already have the preference "memory-inspector.refreshOnStop", with which this new preference clashes a bit. [...]
So that we could have, for now off onStop afterDelay + the delay preference.

You definitely raise a very interesting point! My reasoning for a dedicated boolean flag (auto refresh enabled) was that I wanted to give the user the option to disable the auto-refresh without the need to change (and lose) the value to something "invalid" (like -1). So in my mind auto refresh and refresh on stop were definitely not mutually exclusive. I don't know if there really is a use case for onStop + afterDelay but maybe @jreineckearm has some experience with this.

Curiously, the refresh on stop option is one of the few "global" setting that we have, i.e., it cannot be changed on a per-view basis. I wonder how that should play out when we combine it with the auto-refresh which I believe should definitely remain on a per-view basis since multiple open views might otherwise flood the DAP with requests. What would you expect to happen if you switch globally from afterDelay to onStop? Should we automatically disable the checkbox and input field for the auto refresh UI? Should it still be enabled (even the checkbox) but simply not have an effect? I feel like we need to carefully think about that as otherwise this might get confusing.

martin-fleck-at · 2024-03-22T16:13:08Z

(force-pushed to solve merge conflict)

jreineckearm · 2024-03-22T16:18:07Z

@colin-grant-work , @martin-fleck-at , a couple of very good points. Let's not rush into merging this one before the release and spend a little more though on it. I'll get back to this next week.

colin-grant-work · 2024-03-22T16:21:41Z

Curiously, the refresh on stop option is one of the few "global" setting that we have, i.e., it cannot be changed on a per-view basis.

I suggest that we allow it - or the new preference that combines it with auto-refresh - to be configured on a per-view basis, as you've done in your PR. Then there can't be a conflict: the options would be encapsulated in a single piece of configuration, and that configuration would configurable in the usual places (globally and per view).

Should we automatically disable the checkbox and input field for the auto refresh UI? Should it still be enabled (even the checkbox) but simply not have an effect?

I think we have a good model for this in the files.autoSave preference: the files.autoSaveDelay takes effect only if files.autoSave is set to afterDelay and is ignored otherwise. I think with a minimum of documentation, it will make sense to users that memory-inspector.autoRefresh.refreshRate only takes effect if memory-inspector.autoRefresh.behavior (or whatever name we choose) is set to afterDelay.

martin-fleck-at · 2024-03-22T16:28:28Z

Curiously, the refresh on stop option is one of the few "global" setting that we have, i.e., it cannot be changed on a per-view basis.

I suggest that we allow it - or the new preference that combines it with auto-refresh - to be configured on a per-view basis, as you've done in your PR. Then there can't be a conflict: the options would be encapsulated in a single piece of configuration, and that configuration would configurable in the usual places (globally and per view).

Ah yes, I somehow misinterpreted your previous statement, having all as a per-view basis that absolutely makes sense. Thank you for the clarification!

martin-fleck-at · 2024-03-25T13:59:01Z

Just throwing that one out there as well: Currently we always refresh the memory inspector when the view state (active/focussed or visible) changes and the view is visible. That can result in quite a few refreshes as during debugging the code editor usually gets focus so clicking between the code editor and the memory inspector also triggers a refresh every time. So while we are at it: Should we also introduce a On Focus Change option?

colin-grant-work · 2024-03-25T14:12:49Z

So while we are at it: Should we also introduce a On Focus Change option?

Or maybe a not on focus change :-). But yes, I think if that's currently happening as part of the default behavior (and if it's something that we can prevent from happening), then it would make sense to include it among the options.

jreineckearm · 2024-03-26T10:28:40Z

I agree that we should try to combine the different triggers into a single user visible setting. I think however that a simple drop-down may not be sufficient. It would rather need to be a drop-down with checkbox items (or similar) to selectively allow adding/removing triggers.

Currently we always refresh the memory inspector when the view state (active/focussed or visible) changes and the view is visible.

Is this like in always fetching data from the debug adapter? I am not entirely sure that switching tabs should do a full re-read from the target system. Appreciate though that this may be a result of the WebView not preserving the previous state when hidden for example due to a tab-switch. And that also a debug adapter needs to do its part in caching values where sensible.
I believe however that we shouldn't burden the user with this decision. We may need to investigate how we can get cleverer here. Such cleverness (like only loading visible data) may also help to improve performance.

On onStop vs afterDelay: it really depends on how long the refresh period is for afterDelay. A large refresh period may require an additional onStop update. On the other hand, we also should consider if afterDelay should always be active. Or if it should only refresh while a CPU is running. Traditionally, I am more used to only update while CPU is running. But there were asks in the past to also refresh if the CPU is in stopped state but peripherals and their memory-mapped register interfaces still receiving and processing stimuli from the outside world.

martin-fleck-at · 2024-03-26T13:37:30Z

@jreineckearm @colin-grant-work I pushed an update to the current branch and updated the description. We now have the following options:

On Stop (previously: 'on' for 'refreshOnStop')
On Focus (previously: always implicit on view state change when the view was visible, now on focus)
After Delay (new: every n milliseconds we refresh the view)
Off (previously: 'off' for 'refreshOnStop')

All options are now view-local, i.e., scoped per view. This made it necessary for new events to be sent to the webview (e.g., stopped on a debug session and view state change of a webview). I think in the long run, it is definitely a good idea to have that information within the webview so we can be more fine-grained and smarter with memory updates. The refactoring of the session tracking could also serve nicely as a basis for #97.

As discussed before those options are currently mutually exclusive (enum) but there might be a use case where you actually want to combine them (i.e., they become toggles) - as mentioned by @jreineckearm. Unfortunately, I lack the real world application for this use case, so I'd highly value your input on how we should best proceed on this. I think having agreement on that before merging the PR is key as we are already breaking the previous refreshOnStop setting by replacing it and for sure we want to avoid too many breaking updates between releases [1].

[1] Note I also found a very small UX issue in the settings sub-group rendering which may also break settings #119.

colin-grant-work · 2024-04-03T20:34:13Z

I'll take a look at this tomorrow.

colin-grant-work · 2024-04-05T03:45:11Z

package.json

+      "editor/title": [
+        {
+          "command": "memory-inspector.show",
+          "group": "navigation",
+          "when": "memory-inspector.canRead && (resourceLangId === c || resourceLangId === cpp)"
+        }


Is this related to the functionality elsewhere in the PR? It looks like the aim is to expose the Memory Inspector functionality more prominently, but the resourceLangId is only a so-so proxy for 'is a source file of a source file for a debug session that can read memory'.

You are correct, it is not strictly part of this PR and so I only mentioned it as a "minor" in the PR description

Add title toolbar item for C/C++ file to access open memory inspector

Mainly because I used it a lot to open a memory inspector from a source file. I understand that the condition is far from complete in this regard but since we are only opening an empty memory inspector, I don't see the harm. However, I understand if you want me to move this out of this PR as wel.

I suppose that if I would put it anywhere, I would put it (if possible) on the toolbar of the debug view rather than the file, since the 'memory' and the 'file' don't have a clear correspondence. For now, perhaps separate from this PR so that it can be discussed separately.

Agree that this is one we should address together with other entry points proposed in #52 in separate PRs.
I believe there was a discussion around source editors already.

package.json

src/plugin/session-tracker.ts

src/plugin/memory-webview-main.ts

src/plugin/session-tracker.ts

src/webview/memory-webview-view.tsx

martin-fleck-at · 2024-04-18T11:26:04Z

@colin-grant-work Thank you very much for your feedback! It took me a while to get back to this but I pushed an update now and replied to your comments - some of which may still need some broader discussion but I'm not sure they all need to happen as part of this task.

jreineckearm · 2024-04-22T16:41:13Z

My apologies for only coming back to this now!

I kind of like the idea of condensing the possible triggers to reduce complexity. But I am a little concerned about losing the ability to mix triggers because of flattening them into an enum. Especially if we need to add more in future. I still have no good idea how to realize this instead. My initial reaction would have been to have a kind of dropdown where you can toggle check marks to indicate which triggers are active. But I couldn't find that pattern yet in VS Code configurations. Often, such things seem to be realized by a bunch of checkboxes.... which we worst case have to accept for now. Let me do some more digging if I find something in the design guidelines...

Regarding onFocus: I am curious if we really should expose this as an option. It feels to me like this is a way to work around the fact that a hidden Memory Inspector instance may not be updated when a different trigger occurred. We probably should rather address this or delay such trigger until the window becomes visible next time. Instead of always updating. This can become quite expensive if the debug adapter lacks caching. Especially if "cycling" through views with Ctrl+Tab.

I think afterDelay isn't really the right name for what's happening. The option was supposed to periodically update the window. What's called delay is in fact the delay between periodic updates.

One original thought about the periodic updates was that they are only active while the CPU is running. Given above discussion, this would become the next option to take into account. Assuming we merge this PR first to get periodic updates in. And address the always-periodic-update vs periodic-update-while-running separately. Which almost could be a dropdown with three options by itself (off being the third)....

Will have a chat with Martin about this again tomorrow. But it feels like we need to keep the options separate and revisit if we can handle the onFocus thing better without the need to make it configurable. Let's not forget that we also have the freeze feature to suppress updates if we don't want them.

jreineckearm · 2024-04-23T08:44:52Z

I had another look at applying the auto-save settings scheme to the window refresh. Unfortunately, there are some differences because of which I don't think they apply one-to-one to the memory window updates:

The triggers for auto-save are rather focused on a document/editor (view). They are all tied to a user interaction/modification within that view. If a user stops interaction with the view, one of these trigger options can fire.
One could say that the auto-save triggers "build up" on each other. First you have the afterDelay which fires after inactivity of the user for a while. Then you have the onFocusChange which is more deliberate user action to trigger the update. The next level is to require the onWindowChange which is an even strong user action then just changing focus.
The triggers (or better events) for window updates and target reads are less controllable by the user. They can happen whenever something happens in the target system. This sometimes is out of the user's control.
However, the user may want or even need to configure sensitivity to each of these events.
- For example the onStop event is something I expect a user to disable very rarely. That's normally the point in time all debug views are updated for the majority of target systems. There may only be very rare cases where you need to suppress this.
- The onFocus update should normally be inactive, if exposed at all. Window contents should normally be updated in the background. Reason is that, even if cached, refreshing and re-rendering a large window buffer on each focus change is quite expensive. However, most of the times there is no reason for that. Even worse: if a target architecture doesn't like memory reads while the CPU is running, or if memory reads while running return inconsistent data (for example if an MMU is in play), this may do more harm than good. But as I understand, these refreshes do happen at the moment on a focus change? I think this needs change.
- Looking at periodic updates, you may want to limit that to a certain CPU state (CPU running or not running) which would give the support for this event already three states to consider. See above about supporting memory reads while CPU runs.

I believe we should keep the trigger/event configuration on the level it was before rather than trying to condense them into a flat list.

Should the settings be only global or also local? I believe also local. Even if I am worried about the advanced window settings getting more and more these days. But this could be handled by a redesign of the in-window settings that distinguishes between common settings and expert settings.

colin-grant-work · 2024-04-25T17:10:38Z

I'm not sure exactly which state you're referring to when you say

I believe we should keep the trigger/event configuration on the level it was before rather than trying to condense them into a flat list.

Do you just mean that you would like to have two configurations (or three, more likely):

onStop: boolean
afterDelay: boolean
delay: number

And not any form of onViewStateChanged?

If so, it sounds like you have strong feelings about it, and I can live with that arrangement.

- Rework 'refreshOnStop' boolean setting to 'autoRefresh' enumerable -- On Stop (previously: 'on' for 'refreshOnStop' -- On Focus (previously: always implicit on view state change) -- After Delay (new) -- Off (previously: 'off' for 'refreshOnStop') - On Stop -- Rework global setting to be local for each Memory Inspector -- Listen to debug session stopped event and propagate to view - On Focus -- Rework implicit refresh update to option in setting -- Listen to view state changes and propagate to view - After Delay -- New option to explicitly define a delay when re-fetching the memory -- Minimum: 500ms, default: 500, input step size: 250ms Refactoring: - Split debug session tracking into dedicated class with session events -- Convert debug events into session events with additional data - Split context tracking from memory provider into dedicated class - Move manifest to common for default values and avoid duplication -- Align 'Min' with 'Minimal' value from manifest Minor: - Add title toolbar item for C/C++ file to access open memory inspector - Improve debugging experience by using inline source maps - Align creation of option enums to use const objects - Additionally guard 'body' on debug responses for safety - Avoid functional React state update where unnecessary Fixes eclipse-cdt-cloud#91

- Improve wording in setting descriptions - Move shared types into common area instead of using type imports - Fix 'fetchMemory' not returning the correct promise

Also remove the show memory inspector toolbar item for C/C++ files

martin-fleck-at · 2024-04-29T14:24:06Z

@colin-grant-work @jreineckearm I rebased the PR and added another commit which separates the options again. We now have Refresh On Stop with on/off as before and Periodic Refresh with always/while running/off. As suggested, I removed the explicit option for On Focus again. I think now everything should be as expected. If you have some time to have another look, I'd really appreciate it.

jreineckearm · 2024-05-04T06:23:26Z

@colin-grant-work , @martin-fleck-at : and apologies again for taking a while to come back to this.

Do you just mean that you would like to have two configurations (or three, more likely):
onStop: boolean
afterDelay: boolean
delay: number

Correct (afterDelay now periodicRefresh): My main concern is the mutual exclusiveness which the flattened enum would impose.

And not any form of onViewStateChanged?

Correct, we should carefully review if we can improve the window to deal with this without introducing another option. For example by tracking a dirty state based on refresh settings and doing an update when the window comes back to the foreground next time, which clears the dirty state. Or by updating windows in the background (pro: capture and keep the state as configured by refresh setting, con: potential target reads that are not needed if the window doesn't come back to the foreground until the next refresh trigger).
I believe this requires more careful thought and discussion about the best strategy. Also, we should aim for a separate PR for this to encapsulate the involved complexity change.

If so, it sounds like you have strong feelings about it, and I can live with that arrangement.

Well...kind of. I'd love to get these enhancements in to test them with users. :-)
If we see that the approach doesn't work with users, then we can revisit to flatten the options.

jreineckearm

Tested the changes, works nicely and behavior is as expected. Awesome work, @martin-fleck-at !

colin-grant-work · 2024-05-06T03:10:49Z

package.json

          ],
          "default": "on",
-          "description": "Refresh memory views when debugger stops"
+          "description": "Refresh Memory Inspectors when the debugger stops"


Suggested change

"description": "Refresh Memory Inspectors when the debugger stops"

"description": "Refresh Memory Inspector windows when the debugger stops"

colin-grant-work · 2024-05-06T03:11:48Z

package.json

+            "Do not automatically refresh after the configured delay"
+          ],
+          "default": "off",
+          "description": "Refresh Memory Inspectors after the configured delay"


Let's include the link to the other preference here as well. That'll be easier for users to click on and make the connection more obvious.

Good idea, I'll add the link!

colin-grant-work · 2024-05-06T03:16:30Z

src/common/messaging.ts

+export interface ViewState {
+    active: boolean;
+    visible: boolean;


This interface is passed around, but I don't see that its values are ever used?

Yeah, good catch! It is a remnant of the code we had for the focus change, i.e., the code that is currently commented:

// activate the code below if you want to refresh the memory when the view becomes active (focussed) // const viewStateChange: Change<ViewState> = { from: from.viewState, to: current.viewState }; // if (hasChangedTo(viewStateChange, 'active', true)) { // this.fetchMemory(); // }

I'll remove the view state related code and remove the commented code as well. We'll always have the PR to look up any history, it does not need to be in the submitted change.

colin-grant-work · 2024-05-06T03:19:47Z

src/plugin/memory-provider.ts

-                    }
-                    contributedTracker?.onWillReceiveMessage?.(message);
-                }
+                onWillStartSession: () => contributedTracker?.onWillStartSession?.(),


I think that this is now equivalent to return contributedTracker since we don't actually do any work of our own here.

I had to adapt the return type of the function but you are of course correct. I'd even say that it is more correct now as we simply return undefined if we do not have an adapter tracker instead of an empty adapter tracker that never does anything. I really like this.

colin-grant-work · 2024-05-06T03:20:40Z

src/plugin/memory-provider.ts

            // We only send out a custom event if we don't expect the client to handle the memory event
            // since our client is VS Code we can assume that they will always support this but better to be safe
            const offset = response?.offset ? (args.offset ?? 0) + response.offset : args.offset;
            const count = response?.bytesWritten ?? stringToBytesMemory(args.data).length;
-            this._onDidWriteMemory.fire({ memoryReference: args.memoryReference, offset, count });
+            // if our custom handler is active, let's fire the event ourselves
+            this.sessionTracker['fireSessionEvent'](session, 'memory-written', { memoryReference: args.memoryReference, offset, count });


The bracket access of fireSessionEvent here is a bit suspicious.

I was not sure whether to make the method public out of habit but since this is a VS Code extension and not a framework, there is a lower risk of exposing "internal" methods. I made the method public since we actually use it from somewhere else, thanks!

colin-grant-work · 2024-05-06T03:25:03Z

src/plugin/memory-webview-main.ts

+        let memoryOptions = initialOptions;
        const disposables = [
-            this.messenger.onNotification(readyType, () => {
-                this.setInitialSettings(participant, panel.title);
-                this.setSessionContext(participant, this.memoryProvider.createContext());
-                this.refresh(participant, options);
-            }, { sender: participant }),
-            this.messenger.onRequest(setOptionsType, o => {
-                options = { ...options, ...o };
-            }, { sender: participant }),
+            this.messenger.onNotification(readyType, () => this.initialize(participant, panel, memoryOptions), { sender: participant }),
+            this.messenger.onRequest(setOptionsType, newOptions => { memoryOptions = { ...memoryOptions, ...newOptions }; }, { sender: participant }),


I'm not sure I understand what's going on here. We reassign the memoryOptions when a setOptions message is requested, but we only use it when we call this.initialize in response to the readyType notification, which should only happen once and should happen before the view sends a setOptions message. Under what circumstances would we used the reassigned value of memoryOptions?

Hmm... it's been too long and I don't really remember why I did that. May have had something to do with the view state changes that may happen earlier? Not sure but I can no longer produce any issues with the previous code, so I restored it.

colin-grant-work · 2024-05-06T03:34:41Z

src/webview/components/options-widget.tsx

@@ -52,6 +53,7 @@ export interface OptionsWidgetProps

 interface OptionsWidgetState {
    isTitleEditing: boolean;
+    isEnablingPeriodicRefresh: boolean;


It looks like this should really be an event handler: when the value is changed to not off, we want to focus the interval input. Alternatively, it isn't necessary: the interval input is always available, so the user can focus it any time they want. This is different from the title editing input, whose appearance is controlled by a state value and into which we want to pass focus as soon as it appears - i.e. in the other case, a state change is the event we're responding to.

I agree with you, I thought it was nice UX but it might be a bit much, especially if the user already configured the delay to what they want once. I therefore removed the auto-focus on the input again.

colin-grant-work · 2024-05-06T03:37:00Z

src/webview/memory-webview-view.tsx

+        // activate the code below if you want to refresh the memory when the view becomes active (focussed)
+        // const viewStateChange: Change<ViewState> = { from: from.viewState, to: current.viewState };
+        // if (hasChangedTo(viewStateChange, 'active', true)) {
+        //     this.fetchMemory();
+        // }


It looks like this is where the view state would be accessed. Given that we've decided not to include this as an option available to user, do we want to include this code?

Initially I wanted to keep it until we all agreed on the options we want to have. As we did that now, I removed all view state-related code to not pollute our code base, thank you for catching that!

planger · 2024-05-07T11:11:54Z

@colin-grant-work Thank you very much for your feedback! Just as a heads-up, we'll look into it early next week, as many of us are out of office this week. Thanks!

- Improve settings descriptions in package.json - Get rid of obsolete ViewState and view state-related code - Properly return provider result for contributed trackers - Make 'fireSessionEvent' public as we access it from other classes - Avoid auto-focusing the delay input field if we change options

martin-fleck-at · 2024-05-14T11:23:07Z

@colin-grant-work @jreineckearm I merged the master into this and resolved some conflicts and then added the PR feedback on top of that. It would be great if someone could have another look to see if everything still works properly. Thank you!

jreineckearm · 2024-05-14T14:38:23Z

Still works as expected after today's updates.

martin-fleck-at force-pushed the issues/91 branch from 70acba8 to f58701c Compare March 22, 2024 16:13

martin-fleck-at force-pushed the issues/91 branch from f58701c to 9322f7a Compare March 26, 2024 13:25

martin-fleck-at force-pushed the issues/91 branch from 9322f7a to dc631e7 Compare March 27, 2024 09:46

colin-grant-work reviewed Apr 5, 2024

View reviewed changes

martin-fleck-at force-pushed the issues/91 branch from cf8befb to 96d8750 Compare April 18, 2024 11:25

martin-fleck-at added 3 commits April 29, 2024 11:07

PR Feedback

b04a3cf

- Improve wording in setting descriptions - Move shared types into common area instead of using type imports - Fix 'fetchMemory' not returning the correct promise

PR Feedback: Separate refresh options and remove 'On Focus'

b6da870

Also remove the show memory inspector toolbar item for C/C++ files

martin-fleck-at force-pushed the issues/91 branch from 96d8750 to b6da870 Compare April 29, 2024 14:22

planger requested a review from colin-grant-work May 3, 2024 15:57

jreineckearm approved these changes May 4, 2024

View reviewed changes

jreineckearm mentioned this pull request May 4, 2024

Enable extenders to overwrite default settings #129

Merged

1 task

colin-grant-work reviewed May 6, 2024

View reviewed changes

martin-fleck-at added 2 commits May 14, 2024 12:25

Merge remote-tracking branch 'upstream/main' into issues/91

9782d0d

colin-grant-work approved these changes May 14, 2024

View reviewed changes

planger merged commit e44ca7f into eclipse-cdt-cloud:main May 14, 2024
5 checks passed

	"description": "Refresh Memory Inspectors when the debugger stops"
	"description": "Refresh Memory Inspector windows when the debugger stops"

Add auto-refresh capabilities for memory inspector windows #115

Add auto-refresh capabilities for memory inspector windows #115

Conversation

martin-fleck-at commented Mar 22, 2024 • edited Loading

How to test

Review checklist

Reminder for reviewers

colin-grant-work commented Mar 22, 2024

martin-fleck-at commented Mar 22, 2024

martin-fleck-at commented Mar 22, 2024

jreineckearm commented Mar 22, 2024

colin-grant-work commented Mar 22, 2024

martin-fleck-at commented Mar 22, 2024

martin-fleck-at commented Mar 25, 2024 • edited Loading

colin-grant-work commented Mar 25, 2024

jreineckearm commented Mar 26, 2024 • edited Loading

martin-fleck-at commented Mar 26, 2024

colin-grant-work commented Apr 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martin-fleck-at commented Apr 18, 2024

jreineckearm commented Apr 22, 2024

jreineckearm commented Apr 23, 2024 • edited Loading

colin-grant-work commented Apr 25, 2024

martin-fleck-at commented Apr 29, 2024

jreineckearm commented May 4, 2024 • edited Loading

jreineckearm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colin-grant-work May 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colin-grant-work May 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colin-grant-work May 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

planger commented May 7, 2024

martin-fleck-at commented May 14, 2024

jreineckearm commented May 14, 2024

martin-fleck-at commented Mar 22, 2024 •

edited

Loading

martin-fleck-at commented Mar 25, 2024 •

edited

Loading

jreineckearm commented Mar 26, 2024 •

edited

Loading

jreineckearm commented Apr 23, 2024 •

edited

Loading

jreineckearm commented May 4, 2024 •

edited

Loading

colin-grant-work May 6, 2024 •

edited

Loading

colin-grant-work May 6, 2024 •

edited

Loading

colin-grant-work May 6, 2024 •

edited

Loading