probe-rs / vscode

VSCode debug extension for probe-rs. It uses the MS DAP protocol to communicate directly with the probe (via probe-rs), and supports basic command line debugging in addition to VSCode UI.
https://probe.rs/
Other
65 stars 5 forks source link

RTT Terminals won't show up #87

Closed hyx0329 closed 4 months ago

hyx0329 commented 4 months ago

Extension version: 0.23.0 VSCode version:

No RTT terminal is opened even when RTT is configured correctly.

It seems there's a mistake in createRttTerminal: https://github.com/probe-rs/vscode/blob/23f7b8a434027754d34864154dcb880da2924e18/src/extension.ts#L200-L205

The index will be a string('distinct') rather than a number, while getting element of a list(rttTerminals) requires a number. This causes an exception raised.

I'm not familiar with TS but I've tested the following patch:

diff --git a/src/extension.ts b/src/extension.ts
index ca793ef..0ea8c78 100644
--- a/src/extension.ts
+++ b/src/extension.ts
@@ -197,7 +197,7 @@ class ProbeRSDebugAdapterServerDescriptorFactory implements vscode.DebugAdapterD
                     name: channelName,
                     pty: channelPty,
                 };
-                for (let index in this.rttTerminals) {
+                for (let index = 0; index < this.rttTerminals.length; index++) {
                     var [formerChannelNumber, , ,] = this.rttTerminals[index];
                     if (formerChannelNumber === channelNumber) {
                         this.rttTerminals.splice(+index, 1);

Then the RTT terminal comes up.

Yatekii commented 4 months ago

Hmm the two should def. not behave differently :/

hyx0329 commented 4 months ago

Let me see... Oh! It seems let ... in will also iterate over the properties of the object. Look at this: Screenshot from 2024-03-12 20-24-00 index is the string distinct XD

noppej commented 4 months ago

@hyx0329 This is a good catch, and reading about it a bit more about TS handling lists, it kinda makes sense. I personally think Typescript should be less 'subtle' about this.

Do you want to PR a fix, or would you prefer one of us to just apply your patch?