-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Attempt at regex for validation #2148
Conversation
I don't think you should able to do |
but if you do |
added a couple more tests |
We only use start. We don't care about the range. Then supporting start and end is misleading. |
Ok fine; I added a few more tests on this branch. If you can pass all my tests then your code is fine 😊 The regex thing was just a quick 15-min experiment to see if it was clearer, because this task feels very much like what regexes were designed for, but after implementing it I'm not sure I'm convinced the regex approach is any clearer 😅 I think the clearest would be a parser, but that is most definitely overkill 😄 |
I started with a regex, but I thought it became to hard to read and reason about that I went to the whitelist approach which is much simpler. You can even print the whitelist if you want :D A parser would be nice because then we can complain about using wild card with no "named scope" and similar problems. For now I think it might be overkill. I don't think we should support |
Makes sense 😄
Sorry what are the 'no "named scope" and similar problems'?
Oh ok I had thought we were moving in the direction of the |
A named scope capture like
I can't remember a discussion like that, but I can see the logic behind it. So we should allow any capture that starts with a underscore but they can't have any relationships or positions? |
Incorporated changes into #2147 |
Certainly a lot less code. Also allows things like
statement.startOf
, which should be allowed I think. We have zero-width captures, eg for name of start tag with react fragmentsChecklist