Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Ensure
addActivityItemStream()
doesn't lose Output
vs Error
str…
…eam specialization (#4848) Addresses #4519 In `addActivityItemStream()`, either a `ActivityItemOutputStream` or `ActivityItemErrorStream` always goes in, but it was possible for a "generic" `ActivityItemStream` to come out here when we make one from the `remainderText` after the last newline: https://github.com/posit-dev/positron/blob/e51332d5baa59b72f5e8b28861bbb751f25452ea/src/vs/workbench/services/positronConsole/browser/classes/activityItemStream.ts#L118-L136 That generic `ActivityItemStream` would get set here: https://github.com/posit-dev/positron/blob/c9504d307c686ce2deefdc6887ad2f7b044dd2e9/src/vs/workbench/services/positronConsole/browser/classes/runtimeItemActivity.ts#L110-L116 And then pushed onto `this._activityItems` https://github.com/posit-dev/positron/blob/c9504d307c686ce2deefdc6887ad2f7b044dd2e9/src/vs/workbench/services/positronConsole/browser/classes/runtimeItemActivity.ts#L138-L139 The problem is that our Console code doesn't know how to deal with a generic `ActivityItemStream`. It must be a specialized `ActivityItemOutputStream` or `ActivityItemErrorStream`! https://github.com/posit-dev/positron/blob/c9504d307c686ce2deefdc6887ad2f7b044dd2e9/src/vs/workbench/contrib/positronConsole/browser/components/runtimeActivity.tsx#L46-L49 If a generic `ActivityItemStream` slipped through, the output would never appear in the Console, making it look "dropped", as reported in #4519. --- I like how we have a generic `ActivityItemStream` class that we specialize for the output/error types, so I've fixed this bug by using [polymorphic `this`](https://www.typescriptlang.org/docs/handbook/advanced-types.html#polymorphic-this-types) to allow `addActivityItemStream()` to take and return objects of the correct specialized type. Ideally we'd simply be able to use `new this.constructor()` in place of `new ActivityItemStream()` to use the polymorphic constructor when we need to create new objects, but due to some [well known weirdness](microsoft/TypeScript#3841) in typescript, you have to manually cast it to the constructor type, so `newActivityItemStream()` wraps that up. I've also made the `ActivityItemStream` constructor `protected` to avoid us using it by accident in the codebase. We should _always_ be creating `ActivityItemOutputStream` or `ActivityItemErrorStream` instead. Before 😢 https://github.com/user-attachments/assets/b1854d9e-94eb-4952-aead-bebcf5c5d6a2 After 🥳 https://github.com/user-attachments/assets/4a76974c-6862-4e84-a296-620717ddb8da
- Loading branch information