-
Notifications
You must be signed in to change notification settings - Fork 730
Improved support for link references #1531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add unified `definition` field to ResourceLink interface with utility methods - Update MarkdownLink.analyzeLink to parse target/section from definition URLs - Implement TextEdit array overload with VS Code-compatible reverse order - Updated definition generation to be more dynamic and to not need placeholders
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances Foam's link handling by adding support for reference-style link definitions, unifying how both wikilinks and standard markdown links handle references. The changes eliminate the need for [[//begin]]
and [[//end]]
placeholder markers by implementing more dynamic definition management.
- Reference-style link support: Both wikilinks and markdown links can now use reference-style definitions
- Unified reference handling: Consolidated resolution logic across different link types through a common definition field
- Code cleanup: Removed deprecated
[[//begin]]/[[//end]]
markers and renamed reference field to definition throughout
Reviewed Changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
readme.md | Removed deprecated [[//begin]]/[[//end]] markers from documentation |
packages/foam-vscode/test-data/scaffold/*.md | Cleaned up test files by removing old reference markers |
packages/foam-vscode/src/test/test-utils.ts | Updated test helper to support new definition structure |
packages/foam-vscode/src/features/*.ts | Updated workspace symbol provider and commands to work with new definition handling |
packages/foam-vscode/src/core/services/*.ts | Enhanced markdown parser and provider to support reference-style links |
packages/foam-vscode/src/core/model/note.ts | Added ResourceLink helper methods and definition field to links |
packages/foam-vscode/src/core/janitor/*.ts | Updated link reference generator to handle multiple edits |
docs/**/*.md | Removed old reference markers from documentation files |
Comments suppressed due to low confidence (1)
packages/foam-vscode/src/features/commands/janitor.ts:133
- Missing closing brace for the forEach loop. The forEach callback function is not properly closed.
});
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
}); | ||
|
||
it('should apply multiple TextEdits in reverse order (VS Code behavior)', () => { | ||
// This test shows why reverse order is important for range stability |
Copilot
AI
Oct 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test name and comment mention 'VS Code behavior' but the implementation comment states it matches VS Code behavior without providing a reference. Consider adding a link to VS Code documentation or specification that describes this behavior.
// This test shows why reverse order is important for range stability | |
// This test shows why reverse order is important for range stability | |
// See: https://github.com/microsoft/vscode/blob/1.87.0/src/vs/editor/common/model/textModel.ts#L1242-L1260 | |
// (VS Code applies multiple text edits in reverse order to maintain range stability) |
Copilot uses AI. Check for mistakes.
[//begin]: # "Autogenerated link references for markdown compatibility" | ||
[Backlinking]: docs/user/features/backlinking.md "Backlinking" | ||
[//end]: # "Autogenerated link references" | ||
[Backlinking]: docs/user/features/backlinking.md 'Backlinks' |
Copilot
AI
Oct 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent link title - changed from 'Backlinking' to 'Backlinks', but the original 'Backlinking' matches the filename and is likely correct.
[Backlinking]: docs/user/features/backlinking.md 'Backlinks' | |
[Backlinking]: docs/user/features/backlinking.md 'Backlinking' |
Copilot uses AI. Check for mistakes.
This PR adds support for reference-style link definitions in Foam, unifying how both wikilinks and standard markdown links handle link references. (Fixes #1116)
It also removes the need for
[[//begin]]
and[[//end]]
placeholders as now definitions are more dynamically managed. (Fixes #1504)Changes