Fix default view container not registered#806
Conversation
|
Thanks for picking this up so quickly! One concern about the control flow — as written I think it still falls through to the original crash in the case that actually bites a partial setup (no const viewContainer = this.getViewContainer(key);
const container = viewContainer || this.getDefaultViewContainer();
if (!viewContainer) {
collector.warn(/* "…added to 'Explorer'." */);
} else if (!container) { // only reached when viewContainer is truthy → container === viewContainer → never falsy here
collector.warn(/* "Default view container does not exist…" */);
return;
}
// …proceeds to use `container`When I think the const viewContainer = this.getViewContainer(key);
const container = viewContainer || this.getDefaultViewContainer();
if (!container) {
collector.warn(localize('DefaultViewContainerDoesnotExist', "Default view container does not exist and all views registered to '{0}' will not be added to the UI.", key));
return;
}
if (!viewContainer) {
collector.warn(localize('ViewContainerDoesnotExist', "View container '{0}' does not exist and all views registered to it will be added to 'Explorer'.", key));
}Happy to verify your branch against our repro (Codex/Claude on a monaco partial setup with no Explorer registered) — that's exactly the no-default-container path. Once it covers that case I'll close #805 in favour of this. Thanks! |
|
Ok, I did it too fast! |
74bbdb7 to
1b351a9
Compare
|
What about now? |
|
To save a round-trip on why this slips through: stock code-server / VSCodium-web ship the real Explorer container, so
So the |
|
Did your llm even have a look at the updated commit? |
|
Yes — the updated commit is correct. Making |
No description provided.