search

jrmoulton / tree-sitter-slint

A Tree Sitter parser for slint
MIT License
3 stars 2 forks source link

Support the two way binding operator "<=>" #4

Closed Decodetalkers closed 2 years ago

Decodetalkers commented 2 years ago

Sorry a mistake , set me as draft

Decodetalkers commented 2 years ago

Finished maybe

jrmoulton commented 2 years ago

Thanks for the PR! It looks like this adds support for two way binding but only for callback aliases. There is probably a better way to get support for two way binding in all situations. A while ago I actually wrote a test suite for this parser but it didn't get merged here. I just pushed some commits with that test suite. I'll also add some CI so that the grammer.js file and tests are the only ones that need to be merged.

With the changes I just pushed your changes to the grammer.js file are good to go and don't break any tests. If you could either rebase and only commit the grammer.js file or just open a new PR with just the grammer.js file that would be awesome. After rebasing if you just want to merge you're changes to grammer.js as is that is good with me or you could add support for '<=>' in all situations. Either is good with me!

jrmoulton commented 2 years ago

Actually never mind about rebasing. These changes are good to go and I can merge them or if you want we can mark this as a draft until there is full support for the two way binding

Decodetalkers commented 2 years ago

Emm, I want to know , except for callback ,does any place else use the <=> operator? I used to think that only callback use it..

jrmoulton commented 2 years ago

Yeah. Here in the lang-ref document there is more information. https://github.com/slint-ui/slint/blob/master/docs/langref.md#two-way-bindings

Decodetalkers commented 2 years ago

@jrmoulton finished, please review it, thanks

Decodetalkers commented 2 years ago

And if your tests need help?

Decodetalkers commented 2 years ago

Hello? can you review it now?

Decodetalkers commented 2 years ago

@jrmoulton ping

jrmoulton commented 2 years ago

Hello? can you review it now?

@Decodetalkers Did you see the review that I left about a week ago? I left a series of comments and suggestions.

Decodetalkers commented 2 years ago

Hello? can you review it now?

@Decodetalkers Did you see the review that I left about a week ago? I left a series of comments and suggestions.

I have already make changes

jrmoulton commented 2 years ago

I left comments and requests on the changes as a review and those haven't been addressed. One is the issue with pattern for identifiers. It actually needs to be a regex like 

   _identifier: ($) => /[a-zA-Z_]([a-zA-Z_0-9-]?)+/,

I left more details as a review

Decodetalkers commented 2 years ago

I left comments and requests on the changes as a review and those haven't been addressed. One is the issue with pattern for identifiers. It actually needs to be a regex like

   _identifier: ($) => /[a-zA-Z_]([a-zA-Z_0-9-]?)+/,

I left more details as a review

Fixed

jrmoulton commented 2 years ago

Awesome! This looks great.

Thanks for the contribution!