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

Tree-sitter fixes (1.116 edition) #968

Merged

Conversation

savetheclocktower
Copy link
Contributor

This is a great one! I'm declaring victory in our experiment from #878. We are switching Markdown parsers. The newer one is actively maintained and has fixed the one bug that was standing in the way of our original adoption.

I've also thrown in some TypeScript fixes, and I expect for there to be a few more by the time I'm done.

…when defining hyperlink injection.
…with one based on a better parser.

The `ikatyang` parser is great… until multi-byte input comes into play, or some other construct it doesn't understand. It doesn't have any way to recover when it hits those cases; it just throws exceptions, and WASM can't catch them.

The `MDeiml` parser for Markdown had one deal-breaking issue (tree-sitter-grammars/tree-sitter-markdown#92), and once that got solved, it emerged as the clear victor.

Several people have been using this grammar as `language-markdown-alpha` on the repository, and none of them have encountered any problems of the sort they did with the `ikatyang` parser.
@savetheclocktower savetheclocktower marked this pull request as draft April 4, 2024 06:21
…so they don’t also apply to namespaced TSX tags.
@savetheclocktower savetheclocktower marked this pull request as ready for review April 8, 2024 16:44
@savetheclocktower
Copy link
Contributor Author

Taking this one out of draft a week early because of how big it is (the replacement Markdown grammar).

Copy link
Member

@Spiker985 Spiker985 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without testing, LGTM - when reading through there were no obvious "WTF"s

I saw the comments in src/wasm-tree-sitter-grammar, do we know why that file has an activation/deactivation? It's not like that file is a grammar unto itself, so there shouldn't be any need for [de]activations, right?

@savetheclocktower
Copy link
Contributor Author

I saw the comments in src/wasm-tree-sitter-grammar, do we know why that file has an activation/deactivation? It's not like that file is a grammar unto itself, so there shouldn't be any need for [de]activations, right?

It's a grammar, but it's not a package, so you're right that it's not entirely clear why it has activate and deactivate. I had copied those over from tree-sitter-grammar.js, but I'm not sure they were ever used in that class either.

@Spiker985
Copy link
Member

Hmm...do we know what happens if we remove them...?

@savetheclocktower
Copy link
Contributor Author

Hmm...do we know what happens if we remove them...?

No, but I'll leave that until next month's PR. :)

Copy link
Member

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am willing to sign off on that last little Python-related commit there, seems reasonable.

(Again, without manual testing or anything.)

@savetheclocktower savetheclocktower merged commit 41d8915 into pulsar-edit:master Apr 14, 2024
104 checks passed
@savetheclocktower savetheclocktower deleted the tree-sitter-april branch April 14, 2024 20:19
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.

3 participants