Extension debug configuration and session fixes #11144
Conversation
github-actions
bot
added
the
needs-area-label
adamint
added
area-extension
and removed
needs-area-label
|
|
||
| // Filter out build artifacts and common excluded directories | ||
| const filteredCandidates = candidates.filter(filePath => | ||
| !this.isBuildArtifact(filePath) && path.basename(filePath).includes('AppHost') |
There was a problem hiding this comment.
Isn't there a property that tells us it's an AppHost?
Are there faster cross-plat ways to do file searches? File watchers are a headache.
There was a problem hiding this comment.
PS C:\dev\aspire-dogfood> findstr /S /I /M "IsAspireHost" .cs
Dogfood\Dogfood.AppHost\Dogfood.AppHost.csproj
There was a problem hiding this comment.
There is a property IsAspireHost but we would have to evaluate it to be sure. This is a heuristic to be used until the CLI implements a better and faster way to get app host information, I'm not too worried about it being rigorous at the beginning.
Same re: file watchers
There was a problem hiding this comment.
How does the CLI do it today?
It actually runs msbuild on all of the csproj to determine whether IsAspireHost is true, and has incorrect logic for single-file apphost (AppHost.cs). I believe the maui extension does actually just check for a hardcoded string in the property, but the current approach is a temporary workaround anyway.
There was a problem hiding this comment.
Are you worried about making this fast?
| return candidates; | ||
| } catch (error) { | ||
| extensionLogOutputChannel.error(`Error computing app hosts for ${folder.name}: ${error}`); | ||
| this._projects.set(folder, null); |
There was a problem hiding this comment.
Do we get any value out of storing null when we couldn't resolve the AppHosts in the workspace? It feels like a sensible structure would be:
- Key exists in Map and is not null/undefined: there's an apphost project here
- Key exists in Map but is null: this workspace has no apphosts (maybe use an empty set here to be more explicit?)
- Key does not exist in Map: there's nothing in the cache
| @@ -26,7 +25,7 @@ export class AspireDebugConfigurationProvider implements vscode.DebugConfigurati | |||
| }); | |||
|
|
|||
| try { | |||
| for (const candidate of await this.computeAppHostCandidates(folder)) { | |||
| for (const candidate of await this._availableProjectsService.getAppHostCandidates(folder)) { | |||
There was a problem hiding this comment.
It seems like computeAppHostCandidates used to throw which would trigger a warning message to the user. Now, all the error handling is encapsulated in the AvailableProjectService and we won't actually get an error surfacing here (and hence no warning to the user). Is that correct/expected?
Changes debug configuration retrieval to provide a best guess of available app host projects and single-file app hosts without calling into the aspire cli, which is too slow on large codebases (like the aspire codebase).
Also ensures that the project path is actually passed to the CLI if specified.
Improves disposal on the extension context.