-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
feat: allow gx to function for markdown links #28630
Conversation
Since the markdown parser is bundled could we use Tree-sitter for this? The downside is that maybe the grammar changes (affecting the node names) but it seems like this could be easier: local current_node = get_node { ignore_injections = false }
while current_node do
local type = current_node:type()
if type == 'link_destination' then
return do_open(vim.treesitter.get_node_text(current_node, 0))
elseif type == 'inline_link' or type == 'image' then
return do_open(vim.treesitter.get_node_text(current_node:named_child(1), 0))
elseif type == 'link_text' or type == 'image_description' then
return do_open(vim.treesitter.get_node_text(current_node:next_named_sibling(), 0))
end
current_node = current_node:parent()
end |
I dunno if there's any problems with that, I'll need to ask the ts squad: @neovim/treesitter |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Syntax aside, is relying on the markdown parser acceptable in this case? |
What is the benefit in using the Markdown parser? This seems easy enough to parse manually. |
@lewis6991 No real reason if it is easy enough to parse manually; that said I do think the code looks a bit simpler (and using the parser would handle cases with escaped |
Also the inline parser is pretty flat so traversing up nodes like that (only needed when there is e.g. an emphasis node inside the link text node) won't be too expensive |
I tried the treesitter suggestion (with some adjustments to get rid of type warnings). It works as expected. I suspect this might be more robust than my initial PR. Thanks for the suggestion. I will leave this up for a bit to see if there are more input on this. |
0766870
to
8cc1769
Compare
228a1bd
to
331d55b
Compare
I see what you mean. The parser thinks the text inside |
Ah I see; but couldn't it be the case that there happens to be some language using some weird |
Theoretically, yes. But I still wonder if the benefit of always enabling this isn't greater than the potential clash with a language that may or may not exist. I certainly can't come up with any. I don't think this decision is irreversible, if we notice it causes any problems for some filetype we can further evaluate and iterate on this. I'm not dead set on either approach though, so input welcome. @justinmk any thoughts? |
Tested locally. It works, but it seems that the parse tree gets "stale". If I use Possibly #28715 is needed, or just need to re-parse. Or maybe we should use |
fc164f3
to
5bfef49
Compare
Would it be possible to export this as a callable function? Or add a way to hook into it? I maintain a plugin which adds support for LSP document links. It has a gx function which opens the link under the cursor or falls back to the default If this change goes in as-is, I no longer have a simple way to fallback to the default. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I've created module |
Future improvements:
|
@dundargoc FYI vscode is using LSP document links to implement this functionality for markdown files https://github.com/microsoft/vscode/tree/main/extensions/markdown-language-features/server |
In other words, `gx` works regardless of where it was used in `[...](https://...)`. This only works on markdown buffers. Co-authored-by: ribru17 <[email protected]>
In other words,
gx
works regardless of where it was used in[...](https://...)
. This only works on markdown buffers.