Feel-ix-343 / markdown-oxide

PKM for the LSP
https://oxide.md
Apache License 2.0
986 stars 16 forks source link

Don’t treat labelled URL links like Obsidian internal links #30

Closed fnuttens closed 7 months ago

fnuttens commented 7 months ago

The LSP emits a warning for every URL that’s labelled.

For example,

[link to website](https://example.org)

Shows Unresolved Reference warning.

Feel-ix-343 commented 7 months ago

Ah thank you!

I was just experiencing this and planning to fix it!

I will remove the unresolved reference for URLs, but I think there should still be get_references functionality for them. I find this useful. What do you think?

fnuttens commented 7 months ago

First, let me thank you for tackling this, I’ve been trying to use Obsidian with vim bindings but since I’m on Helix it’s far from ideal. I have high hopes with markdown-oxide! 🙂

Back on the URL issue, I agree that get_reference is still relevant for them. Maybe that’s a discussion for another issue, but I noticed that the reference count was only taking the FQDN into account. I feel like the path should matter; for example:

- [crossterm#820](https://github.com/crossterm-rs/crossterm/issues/820)
- [helix#5718](https://github.com/helix-editor/helix/issues/5718)
- [helix#7129](https://github.com/helix-editor/helix/issues/7129)

Here the LSP counts 3 uses of the ref, even though each link points to a different path.

Feel-ix-343 commented 7 months ago

Thanks for the compliments!!! That is the exact reason I created this; Make an obsidian-inspired/compatible PKM system for any text editor. I'm happy to see it being used on Helix -- Zed, VSCode, and Online Editors are next! Also note that the completions do not work as intended on helix yet; They don't implement is_incomplete which this LS frequently uses. I opened an issue and gave a temporary fix (can't really even call it a fix actually its so bad) here. (FYI This is all necessary because this LS does filtering and sorting of completions on the server)

And agree about the path for reference. References on URL links actually is not even supposed to be a feature; It just happens based on the regex. All of the behavior right now is completely unintended (it is interesting to see, however).

As for the references, I envision references to a URL with no path as giving all references to that URL with any path. Further, references to a path should give all references to that path + subpaths. This really should not be that hard to implement; Just creating a MarkdownURLLink Referenceable and a MarkdownURLLink Reference then implement referenceable.matches_references and reference.references based on the logic above. Similarly to tags, calling goto_definition on a markdown reference I think should give places only where that exact URL is used, in contrast to lsp_references

I'll give this a shot soon -- or if you want to, you could try!

Also note that the code is going to have a minor refactor soon to make code for completions more maintainable. Most of it will stay the same however; Check out the vault module if you are interested! I am also happy to talk through any questions.

And again, thank you for the comments! Open-source collaboration is the most transparent and engaging form of collaboration in software. I'm so thankful I get to experience it!!!!