-
Notifications
You must be signed in to change notification settings - Fork 678
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
Ada: fixing highlighting for Ada aspects #2125
Conversation
Aspects were introduced in Ada 2012.
9ad800b
to
bec1f56
Compare
Could you elaborate on the rationale for this change? Even if |
I'm not sure how useful In any case, the current parsing is simply wrong when it comes to Ada aspects. Take this code, for example:
Here, the current version of the parser highlights the first name ( |
Sorry for the delay. I'm still not understanding this change well: the Is it possible to recognize the syntax better? Just looking at your example, it looks like a lookahead assertion |
Another example of Ada aspects could be this: procedure Update (X : in out Integer)
with Inline; In this case, the lookahead you're suggesting wouldn't work. |
OK. Then what is the |
I'm not sure. I guess |
Could you please let me know what's the next step from here? |
This seems to be removing a feature in order to fix a bug related to the feature. I'm honestly not sure this is a good approach. It can be if the feature has no chance of being fixed. Does it take a lot of work to fix this fully while retaining the special Name.Namespace special highlighting? If so, I'd rather see the 'import' state removed and a comment that parsing that is too tricky and thus not done. |
Actually, before submitting this pull request, I already tried to come up with a solution that would include full support for Ada aspects. However, it seemed much more complex than anticipated. Today, I tried to find a way to minimize the impact of my PR, but I haven't succeeded in that. To be honest, I doubt there's an easy way to solve this issue without a major refactoring of the Ada lexer. |
SGTM now, but fails testing. Should |
cb6b790
to
f3bd68c
Compare
Sorry, I just tried out removing the import state, but it just makes it worse. (When removing it, names such as |
I'll merge this and add a comment. |
Thanks! |
Aspects were introduced in Ada 2012.