-
Notifications
You must be signed in to change notification settings - Fork 30
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
Misc Conversation #185
Comments
I'll be working on the grammar today. @matter123 did you seen the new tree sitter extension? |
I haven't had a chance to actually use it yet, but it looks pretty nice. I did notice that the syntax highlighting is rough in a few areas.for example parameter highlighting. Do you know if that's a tree sitter issue, a issue with the extension, or a bad theme choice? |
@matter123 I'm pretty sure that's just a minor issue with the extension---it's only 3 days old! We need a C++ expert to come tweak the function that traverses the tree-sitter tree and applies colors: |
@matter123 So I looked into the Atom one a bit ago, because they're missing a lot of scopes. From what I understand the Tree sitter parses the whole thing and basically has its own tree-scopes, then there is an additional file that maps those scopes to the traditional TextMate names. So for example, in Atom the lambda is correctly parsed by the tree sitter, but Atom incorrectly marks the I'm not sure if the C++ tree sitter implementation is currently marking the difference between things like parameter-variables and just normal variable usage. |
@matter123 |
@jeff-hykin unless your planning on doing some more big changes today. I'm going to work on fixing the tests. |
No big changes, I was just going to completely regenerate the tests though since so much has changed. |
Yeah, it's taking me about 7 minutes per fixture so at 267 fixtures that comes out to 31 hours. regeneration is probably needed. |
I added you to the experimental tree sitter repo @matter123 just so you'd have access if you wanted it. https://github.com/jeff-hykin/experimental-tree-sitter/invitations |
@matter123 is there a way to automate the tagging releases? |
Edit: githubs topic on release automation https://github.com/topics/release-automation |
|
Not sure what happened but your tagged version was from 184 commits ago |
sigh haha. How you you make the tags? I just used the "new draft" button on the tags page |
Although I did it after running |
Pressing new draft is all I do. Was the branch you were on happen to be behind? |
Oh I know what happened. You release 1.9 with the new enum pattern then reverted. |
@matter123 I generated the spec files for Objective C/C++ but for some reason all of them are missing the |
|
huh, let me take a look. |
So the issue is that there are more scopes popped off in scopeEnd than are pushed on in scopeBegin
show the problem. |
The reason I added a manual one is so that it exists even inside of other languages, like markdown. |
@jeff-hykin What is the purpose of the hidden portion of scope resolution. And why does it rematch the visible portion of the scope resolution? |
Somehow they are slightly different and to be honest I've forgotten how exactly. I tried to combine them when I fixed the dot-access operator tag |
The most recent push to The new version is however noticeably slower (~18.3s for previous, ~23s for new), I am unsure what the cause is. |
Sounds good, the whole branch is definitely a big WIP. You might want to look at the latest As a side note: when there are grammar-usage bugs, some of them can be really hard to debug, they fail far away from their source with an unrelated error. So I might work on adding more checks, logging, and descriptive error messages before release. Examples:
|
should the grammar complain if an imported grammar's exports end up not being used? as far as infinite recursion in placeholders, do you think it is worthwhile to implement some sort of cycle detection algorithm? I'll see if I can figure out what's going on with dont_match. |
I did make partial progress on this. Idk if Onigurua also will throw an error with numbered backref's and named capture groups, but it probably will since its Ruby based. Idk why that wouldn't be allowed in Ruby either. |
Thanks 👍 I'll probably test it out in the next few days to see exactly what you mean with the versions |
Yeah, we can make a placeholder set {} that is empty at the top level. Then when a placeholder is being resolved it adds the PlaceholderPattern object to the set, and if its already on the set, then its an infinite recursion. |
slightly simpler might be to borrow from dfs mapping and mark each placeholder node as ("not resolved", "resolved", or "resolving") and attempting to resolve a "resolving" placeholder is a loop. Otherwise I belive you would have to clear the set between resolves to prevent false positives on already resolved patterns. |
There might also be a way to do it before any loop/recursion by changing the resolve strategy. Every pattern in the repository, like Something like
|
So looking at the new resolve_placeholder transformation, I am not sure how it is recursive. if Imagine the following grammar: g[:a] = Patterb.new(/a/).then(g[:b])
g[:b] = Pattern.new(/b/).then(g[:c])
g[:c] = Pattern.new(/c/) Prior to calling save the grammar has the following {
a: Pattern.new(/a/).then(PlaceholderPattern(:b))
b: Pattern.new(/b/).then(PlaceholderPattern(:c))
c: Pattern.new(/c/)
} Save_to is then called resolving in the resolve_placeholder plugin running. Edit: does |
I'm not 100% sure
|
This I'm not sure of either, which is partly why I worked on creating |
I don't think it can be frozen, several transformations alter the chain fairly significantly. |
Sorry I meant just top-level. Basically no elements are added/removed/re-ordered on the list, but each can be mutated. |
I pushed the recursive bug on master on the experimental better-c-syntax repo if you want to see the stack overflow bug. The grammar only has 1 pattern in it at the moment. I'm still not sure how its being caused by |
Freezing at the top level should cover any existing plugins but ideally, bailout would be a part of the grammar and that is going to add more names to the grammar. There could be two stages, in the first stage, the pattern objects are immutable but the repository is mutable and in the second stage its the opposite. |
Yeah that might work since the bailout should be independent of any placeholders or other resolutions |
is |
Well a few things happened. (I pushed 1 more change (actually a revert))
The misspelling is important cause it tried to access a non-exported pattern |
I pushed a change that elminates the issue with the recusion in deep clone. With the change this is the new error for better-c-syntax:
|
Based on that doc you sent sent earlier. Should we be using \k for backreferences instead of \g? |
backreferences are |
Does the cpp code have subexpressions? |
I thought \N was problematic for values > 9 so |
just the template angle brackets, but that's planning on being removed? |
the docs say n is valid when (n >= 1) |
Oh yeah I forgot I used it there, maybe that's where I'm seeing the \g from. As a side note, a lot later on, we could probably use subexpressions to compress and maybe optimize the regex for single patterns based on how they reuse patterns inside themself |
Hmm I wonder how you're supposed to match a backreference followed by a digit literal. Non capture groups?
That could create some edge case bugs, depending on how we replace the backreferences
|
Backreferences actually become |
I guess just for future reference:
Has there been any development in the performance loss of applying a pattern inside a capture item in the C highlighter? |
@RedCMD
Sadly no |
This issue is just a place for chat that doesn't necessarily relate to an issue. I'm going to close it immediately so that it doesn't add to the normal issue count, but it can still be used for general conversation.
The text was updated successfully, but these errors were encountered: