Fixed Shared Files in VS-Code + Some Documentation Updates#2454
Fixed Shared Files in VS-Code + Some Documentation Updates#2454
Conversation
…er than the old array.
There was a problem hiding this comment.
Pull request overview
This PR fixes shared file handling in VS Code for multi-project workspaces and updates documentation to reflect the correct configuration format. The fix ensures that when multiple projects include the same directory path, both projects can access documents from that shared location, resolving a critical functionality issue in the VS Code extension.
Key changes:
- Implemented proper shared include path support in
LangiumDocuments.projectDocuments()andIndexManager.projectElements() - Added comprehensive test coverage for the shared include path scenario
- Updated documentation to show the correct
include: { paths: [...] }config structure instead of the incorrectinclude: [...]format
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/language-server/src/workspace/LangiumDocuments.ts | Modified projectDocuments() to filter documents by project folder and include paths using URI string comparison |
| packages/language-server/src/workspace/IndexManager.ts | Modified projectElements() to include documents from both the project's own folder and configured include paths |
| packages/language-server/src/workspace/ProjectsManager.spec.ts | Added test case verifying that multiple projects can share the same include path and access shared documents |
| apps/docs/src/content/docs/dsl/Config/multi-projects.mdx | Updated documentation example to show correct include: { paths: [...] } structure |
| apps/docs/src/content/docs/dsl/Config/index.mdx | Updated documentation to show correct include configuration structure and added details about maxDepth and fileThreshold options |
| return true | ||
| } | ||
|
|
||
| // Check for addtional documents when the config has the `include:paths` property set. |
There was a problem hiding this comment.
Spelling error: "addtional" should be "additional".
| // Check for addtional documents when the config has the `include:paths` property set. | |
| // Check for additional documents when the config has the `include:paths` property set. |
📝 WalkthroughWalkthroughThe PR updates documentation for the Include configuration structure and implements project-aware document inclusion logic in the language server workspace manager. Document filtering shifts from ID-based matching to path-based scope that respects both project folders and configured include paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/language-server/src/workspace/IndexManager.ts (1)
29-34: Simplify the empty array check.The
includePathStrings.length > 0guard is redundant sinceArray.prototype.some()already returnsfalsefor empty arrays.Apply this diff to simplify:
.filter(uri => { const belongsToProject = projects.belongsTo(uri) === projectId - const inIncludePath = includePathStrings.length > 0 && - includePathStrings.some(includePath => uri.startsWith(includePath)) + const inIncludePath = includePathStrings.some(includePath => uri.startsWith(includePath)) return (belongsToProject || inIncludePath) && (!uris || uris.has(uri)) })packages/language-server/src/workspace/LangiumDocuments.ts (1)
95-97: Consider removing redundant empty array check for consistency.Similar to the pattern in
IndexManager.ts, theincludePathStrings.length > 0check is redundant sinceArray.prototype.some()returnsfalsefor empty arrays.- if (includePathStrings.length > 0) { - return includePathStrings.some(includePath => docUri.startsWith(includePath)) - } - - return false + return includePathStrings.some(includePath => docUri.startsWith(includePath))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/docs/src/content/docs/dsl/Config/index.mdx(2 hunks)apps/docs/src/content/docs/dsl/Config/multi-projects.mdx(1 hunks)packages/language-server/src/workspace/IndexManager.ts(1 hunks)packages/language-server/src/workspace/LangiumDocuments.ts(1 hunks)packages/language-server/src/workspace/ProjectsManager.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
pnpm ci:typecheckfor TypeScript type checking across all packages except documentation
Files:
packages/language-server/src/workspace/ProjectsManager.spec.tspackages/language-server/src/workspace/IndexManager.tspackages/language-server/src/workspace/LangiumDocuments.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use oxlint with type-aware rules for linting instead of eslint, and run
pnpm ci:lintto validate all packages except documentation
Files:
packages/language-server/src/workspace/ProjectsManager.spec.tspackages/language-server/src/workspace/IndexManager.tspackages/language-server/src/workspace/LangiumDocuments.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
pnpm fmtto format code with dprint instead of Prettier or eslint for formatting
Files:
packages/language-server/src/workspace/ProjectsManager.spec.tspackages/language-server/src/workspace/IndexManager.tspackages/language-server/src/workspace/LangiumDocuments.ts
packages/*/**/!(node_modules)/**/*.spec.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Unit tests must be located in
packages/*/src/**/*.spec.tsorpackages/*/src/**/__test__/*.spec.tsfiles
Files:
packages/language-server/src/workspace/ProjectsManager.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-12-05T15:36:11.968Z
Learnt from: CR
Repo: likec4/likec4 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T15:36:11.968Z
Learning: Applies to packages/*/**/!(node_modules)/**/*.spec.ts : Unit tests must be located in `packages/*/src/**/*.spec.ts` or `packages/*/src/**/__test__/*.spec.ts` files
Applied to files:
packages/language-server/src/workspace/ProjectsManager.spec.ts
🧬 Code graph analysis (2)
packages/language-server/src/workspace/ProjectsManager.spec.ts (7)
packages/language-server/src/test/testServices.ts (1)
createMultiProjectTestServices(190-256)packages/likec4/src/LikeC4.ts (1)
projectsManager(166-168)packages/language-server/src/workspace/LangiumDocuments.ts (1)
addDocument(28-39)packages/language-server/src/model/fqn-index.ts (1)
documents(59-64)packages/language-server/src/model/model-parser.ts (1)
documents(97-101)packages/language-server/src/model/model-builder.ts (1)
documents(300-302)packages/language-server/src/model/model-locator.ts (1)
documents(47-49)
packages/language-server/src/workspace/IndexManager.ts (1)
packages/language-server/src/LikeC4LanguageServices.ts (2)
project(154-175)projects(105-152)
🪛 GitHub Check: SonarCloud Code Analysis
packages/language-server/src/workspace/IndexManager.ts
[warning] 31-31: The non-empty check is useless as Array#some() returns false for an empty array.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: checks / 🔬 e2e types
- GitHub Check: checks / 📖 docs
- GitHub Check: checks / 🔬 e2e tests
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: checks / ⊞ windows build
🔇 Additional comments (6)
packages/language-server/src/workspace/IndexManager.ts (1)
18-36: LGTM!The project-aware inclusion logic correctly extends URI filtering to include documents from both the project folder and configured include paths. The trailing slash normalization ensures consistent path matching.
apps/docs/src/content/docs/dsl/Config/multi-projects.mdx (1)
68-150: LGTM!The documentation clearly explains the updated
includeconfiguration structure with thepathsarray, and the new scanning options (maxDepth,fileThreshold) are well-documented with sensible defaults and usage tips.apps/docs/src/content/docs/dsl/Config/index.mdx (1)
59-113: LGTM!The documentation updates consistently reflect the new object-based
includeconfiguration structure and provide clear examples for both basic and advanced scanning behavior options.packages/language-server/src/workspace/ProjectsManager.spec.ts (1)
502-543: LGTM!The test thoroughly validates the shared include path scenario:
- Verifies project-specific document ownership
- Correctly allows the shared document to belong to either project or default
- Confirms both projects can access the shared document via
projectDocumentsThis aligns well with the implementation changes in
IndexManager.tsandLangiumDocuments.ts.packages/language-server/src/workspace/LangiumDocuments.ts (2)
76-101: LGTM!The
projectDocumentsmethod correctly implements path-based inclusion logic that aligns with theIndexManager.tschanges. Documents from the project folder are always included, and documents from configured include paths are included when specified.
80-84: The conditional check on line 80 is necessary and should not be removed. WhilefolderUrifor registered projects is normalized viaProjectFolder()to include a trailing slash, the default project'sfolderUriis obtained directly fromWorkspaceManager.workspaceUri(orURI.file('/')as fallback) without going through theProjectFolder()normalization. The defensive check handles both cases correctly.Likely an incorrect or invalid review comment.
| return true | ||
| } | ||
|
|
||
| // Check for addtional documents when the config has the `include:paths` property set. |
There was a problem hiding this comment.
Fix typo in comment.
"addtional" should be "additional".
- // Check for addtional documents when the config has the `include:paths` property set.
+ // Check for additional documents when the config has the `include:paths` property set.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check for addtional documents when the config has the `include:paths` property set. | |
| // Check for additional documents when the config has the `include:paths` property set. |
🤖 Prompt for AI Agents
In packages/language-server/src/workspace/LangiumDocuments.ts around line 94,
the inline comment has a typo: "addtional" should be corrected to "additional";
update the comment text to read "Check for additional documents when the config
has the `include:paths` property set." to fix spelling while preserving intent
and formatting.
Checklist
mainbefore creating this PR.Summary by CodeRabbit
Documentation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.