Skip to content
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

Fix solidity lexer's handling of inline assembly #1040

Merged
merged 5 commits into from
Jan 14, 2025

Conversation

jyjh
Copy link
Contributor

@jyjh jyjh commented Jan 14, 2025

Syntax highlighting in solidity breaks to due the assembly rule improperly pushing the state (it pushes once for the keyword, then again for the '{') This fix merges the opening '{' into the assembly keyword pattern.

Link to chroma playground
Before
After

jyjh added 2 commits January 14, 2025 10:21
Syntax highlighting in solidity breaks to due the assembly rule improperly pushing the state (it pushes once for the keyword, then again for the '{')
fixes assembly to not error out when '.' is used
Copy link
Owner

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind adding a text for this change?

@jyjh
Copy link
Contributor Author

jyjh commented Jan 14, 2025

Sorry, I've now edited the test to include inline assembly.

Copy link
Owner

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!

@alecthomas
Copy link
Owner

Test is failing slightly.

Addes a newline to the end
@jyjh
Copy link
Contributor Author

jyjh commented Jan 14, 2025

That's really strange, cause the test worked locally. It seems there is an added newline at the end despite there not being one in the file?

In any case, I went ahead and added it, it should work now hopefully. Sorry for the inconvenience!

@alecthomas alecthomas merged commit 79621af into alecthomas:master Jan 14, 2025
2 checks passed
@alecthomas
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants