-
Notifications
You must be signed in to change notification settings - Fork 46
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 based parser. #261
Tree-Sitter based parser. #261
Conversation
Thanks for the contribution. I can review it better if the CI tests are running. |
68f317b
to
5912a8f
Compare
It looks like you are missing a commit from master branch:
|
Good work so far! Impressed about the progress. Do you need any help getting tests passing? |
I have some more time this afternoon to hunt down the remaining few issues which shouldn't be that bad now that I have re-learned how to get pytest to give me the information I want. I will let you know then if there are any tests that are particularly thorny, though from looking at the remaining failures it looks like primarily small issues with the interface provided by |
0a155d3
to
e1ecdfd
Compare
Alright it took longer than expected but it seems like all the tests are passing now. There will have to be some changes due to some modifications that the |
tests/test_matlabify.py
Outdated
@@ -138,12 +138,19 @@ def test_classes(mod): | |||
assert isinstance(cls, doc.MatClass) | |||
assert cls.getter("__name__") == "ClassInheritHandle" | |||
assert cls.getter("__module__") == "test_data" | |||
assert cls.bases == ["handle", "my.super.Class"] | |||
assert cls.bases == [("handle",), ("my", "super", "Class")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be changed back to the original. A list of fully-qualified names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea this can absolutely be changed back. I don't think this broke anything and I was thinking it may be useful for later processing but now with fresh eyes I am unconvinced.
tests/test_matlabify.py
Outdated
assert abc.bases == ["ClassInheritHandle", "ClassExample"] | ||
assert abc.attrs == {"Abstract": True, "Sealed": True} | ||
assert abc.bases == [("ClassInheritHandle",), ("ClassExample",)] | ||
assert abc.attrs == {"Abstract": None, "Sealed": None} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the value of the class attribute not parsed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe in this case it was parsed correctly but the matlab code did not assign true and simply used Abstract
on its own. I have not yet integrated the existing code for attribute defaults, but I will for consistency.
import re | ||
|
||
# rpath = "../../../syscop/software/nosnoc/+nosnoc/Options.m" | ||
rpath = "/home/anton/tools/matlabdomain/tests/test_data/ClassWithTrailingCommentAfterBases.m" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded paths are not great after release :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:) yep got rid of this. Was using it for quick and dirty testing.
c929b54
to
6d9405f
Compare
…phinx-contrib#253) * Actually not only enums, quick and dirty start to using textmate parser * a little more boilerplate * property validator parsing * working function parsing without docstrings yet * start enum work * some enum parsing * working parsing for enumeration comments * add handling for block comments to enums * backport enum docstring parsing to properties * remove vestigial file * minor fixes + black
* CI: Testing for latest Sphinx (8.0) * CI: Fix helper class version checking.
…ation issues remain
ceb56d3
to
7da1e65
Compare
Hello @Remi-Gau (also @joeced if you have a chance)! I'd love to have another pair of eyes go over this PR. I am pretty happy with the feature set (I realized this inadvertently addresses #265, see tests involving |
sphinxcontrib/matlab.py
Outdated
table of contents, and can also be used within the | ||
:py:meth:`_toc_entry_name` method. | ||
|
||
This method must not be used outwith table of contents generation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: outwith
-> with out
I looks really nice! I like that tree-sitter looks like it's giving way more structure than pygments. You have my approval :) |
This PR implements an alternative to the pygments/textmate grammar parsers using
tree-sitter
andtree-sitter-matlab
.