Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
(swift) Improved highlighting for functions, initializers, and subscripts #2930
(swift) Improved highlighting for functions, initializers, and subscripts #2930
Changes from 1 commit
a2a27fd
94534b4
9f2b976
f2e95ea
f4f7142
eec1831
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 didn't understand why we needed lookahead at all here.
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.
This is probably a leftover from when I used
contains
. The goal here was to check that the upcoming code is indeed an identifier, but leave matching to the contained rules.Will the
keywords: "_|0"
rule still work with this change?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.
Yes, keywords match the buffer, they do not consume the input.
That whole buffer is fed into the keyword engine for processing. Of course children rules that match can flush the buffer... but it's easy enough to reason about in a simple case with no children.
That's why keywords can get difficult in edge cases, because they are happening absolutely last and at the mercy of the whole ruleset.
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.
No good reason not to consume this that I can see, so we just do that, since it's simpler.
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.
Yes, that should be fine. I didn't consume it just it case I need it to detect a type annotation (which always starts with
:
), but I managed to get proper highlighting without having to do that.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.
We really need look-behind. :( Lots of complexity dealing with these kind of things without it.