Markdown spaces#678
Conversation
Includes tests for Wikilinks, Markdown links, relative paths, root-absolute paths, and resolution priority.
Verifies that the LSP definition handler handles nil or incomplete client capability objects without crashing.
…anic Markdown and WikiLinks are now resolved using an identical three-step hierarchy: 1. Match relative to the current file. 2. Exact match relative to the vault root. 3. Fallback to partial path matching. This priority ensures that relative links are not hijacked by root-level files with the same name. This also fixes the issue where Markdown links requiring extensions were not properly indexed, and ensures consistent link resolution across CLI and LSP functionalities. Additionally, added a nil-pointer check to the LSP definition handler to prevent crashes when resolving definitions for editors that do not provide full TextDocument LinkSupport capabilities.
Includes tests for literal and encoded spaces in both relative and root-absolute Markdown links.
Allows Markdown links to contain literal spaces in their destination, e.g., [label](file name.md), without requiring percent-encoding.
WhyNotHugo
left a comment
WhyNotHugo
left a comment
There was a problem hiding this comment.
This PR tries to tackle two separate issues:
- Links with un-escaped spaces in the target.
- Priority of relative/absolute links.
I'd rather tackle each on separately for ease of review and to understand which changes apply to each one.
| goldmark.WithExtensions( | ||
| gmext.Footnote, | ||
| extensions.WikiLinkExt, | ||
| extensions.MarkdownLinkWithSpacesExt, |
There was a problem hiding this comment.
We only just got rid of our own custom markdown parser: it had way too many edge cases which were broken, and each time we fixed one, another showed up.
I'm very hesitant to re-introduce a custom parser for markdown links.
There was a problem hiding this comment.
That is very understandable! Actually, I did not know this, but the commonmark standard allows for spaces in markdown links if wrapped with <>. E.g. [Link name](<projects/Research Paper/review.md>)
So there is no need for this PR. But maybe this would be something worth adding to the documentation on links?
There was a problem hiding this comment.
In the spec explicitly allows for it, and the parser is rejecting it, we should report this to https://github.com/yuin/goldmark. The author is quite responsive.
|
I'm a little confused. Your issue that you linked is about markdown links not resolving from notebook root -- or at least only resolving with filepaths relative to the current file. As discussed in the issue, this is the standard behaviour of markdown links to other documents. But this PR addresses markdown links with spaces. |
|
Yes, these there two separate issues. But per my comment, this was an oversight on my part regarding the markdown spec. |
3 participants
...
fixes #676
Implements parsing of markdown links with spaces (i.e. ones lacking %-escaping). This breaks with the commonmark standard, but allows for more readable links and parries the wikilinks parsing.