fixed angular path resolution issue - #96
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Angular Zed extension to resolve the Angular language server (and probe locations for TypeScript) relative to the active worktree root, addressing Windows path resolution failures reported in #95.
Changes:
- Resolve
@angular/language-serverfrom the worktree root (with an optional user override) and adjust probe locations used by the language server. - Pass worktree shell environment to the spawned Node process and add default console logging flags.
- Update README with clearer requirements/configuration guidance and bump extension/package version to
0.0.7.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/angular.rs |
Reworks server path/probe resolution to be worktree-relative, adds settings for server path override and memory, and adjusts command env/args. |
README.md |
Documents requirements, configuration options, troubleshooting, and version guidance. |
extension.toml |
Bumps extension version to 0.0.7. |
Cargo.toml |
Bumps crate version to 0.0.7. |
Cargo.lock |
Updates lockfile version entry for the crate. |
Suppressed comments (1)
README.md:132
- This troubleshooting note repeats the hard "outside the supported range" claim for versions above 6.0.3, but the extension doesn’t validate TypeScript versions. Rephrasing to "untested" avoids implying an enforced check.
- **TypeScript resolution errors.** Check your installed TypeScript version with `npx tsc --version` — anything above 6.0.3 is outside the supported range.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I think copilot has some valid opinions, do you think we could fix or test on windows before this gets merged? |
|
@jpike88 do you have a way to test for windows? |
|
I've fired up an ec2 instance of windows server 2025 to test on, will let you know if my testing works, just need 20 mins |
|
works on Windows for me. |
|
@nathansbradshaw Looks good to me. I tested the changes locally, and they fixed the issue on my end. If you're happy with it as well, I think it's ready to merge. Thanks! |
fixes #95
Also, uses the found angular srever path and auto-attempts to resolve the typescript package upwads from there.
Also readme is updated to be a bit more clear. I used Claude Opus 5, and have carefully proof read the output.
@nathansbradshaw please merge! Thanks mate