Skip to content
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

Add wiki-links support (#54) #97

Merged

Conversation

boltlessengineer
Copy link
Contributor

Added basic wiki-link parsing based on Obsidian's wikilink.

@MDeiml
Copy link
Collaborator

MDeiml commented May 12, 2023

Hey, thank you, this looks great. Not sure if it breaks in some edge cases, e.g. nested links, but that is not too important.

I would make it a "non-default" extension for the moment if that is ok with you? That would mean removing the EXTENSION_DEFAULT part in the line you added in common/grammar.js.

It also looks like you broke 2 test cases, see https://github.com/MDeiml/tree-sitter-markdown/actions/runs/4902920851/jobs/8755052765?pr=97. Could you have a look at that?

@boltlessengineer
Copy link
Contributor Author

Hey, thank you, this looks great. Not sure if it breaks in some edge cases, e.g. nested links, but that is not too important.

Because Wiki-link parsing is an extension, prioritizing the wiki-link syntax in nested-link should solve the problem. Example 556 in corpus/spec.txt is showing some nested case.

I would make it a "non-default" extension for the moment if that is ok with you? That would mean removing the EXTENSION_DEFAULT part in the line you added in common/grammar.js.

I actually tried to make this non-default extension, but I couldn't figured out how to build & test as non-default extension. I learned tree-sitter syntax first time while making this PR. Can you give me an example of building & testing non-default extensions? That will be helpful.

It also looks like you broke 2 test cases, see https://github.com/MDeiml/tree-sitter-markdown/actions/runs/4902920851/jobs/8755052765?pr=97. Could you have a look at that?

Those two test cases should be broken when wiki-link option is on. That is intended behavior because Wiki-link is not in CommonMark/GFM spec. Should I remove those test cases?

@MDeiml
Copy link
Collaborator

MDeiml commented May 27, 2023

Sorry for the late answer.

Can you give me an example of building & testing non-default extensions?

You have to set the environment variable corresponding to the extension you want to enable, e.g. EXTENSION_WIKI_LINK=1. How you do this depends on your operating system. On Linux and Mac you should be able to just write EXTENSION_WIKI_LINK=1 npm build, just be sure to run npm clean before that.

Should I remove those test cases?

Hm not to sure how to deal with this, I guess you can remove them for now and I'll think of something later.

@boltlessengineer
Copy link
Contributor Author

@MDeiml I fixed points you said. Can you check this?

@MDeiml
Copy link
Collaborator

MDeiml commented Jun 4, 2023

Thanks a lot, this is great! Build tests were just failing, because my CI used an older version of tree-sitter, but I just updated that to the newer version you were using.

@MDeiml MDeiml merged commit 231f316 into tree-sitter-grammars:split_parser Jun 4, 2023
@MDeiml
Copy link
Collaborator

MDeiml commented Jun 4, 2023

Thanks for your work, I think a lot of people will be happy about this feature. Also sorry I took long with responding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants