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

Don't trim list separators #4596

Merged
merged 3 commits into from
Aug 22, 2024
Merged

Don't trim list separators #4596

merged 3 commits into from
Aug 22, 2024

Conversation

Scott-Guest
Copy link
Contributor

Closes #4592

Previously, we trimmed list separators, presumably under the assumption that #Layout deletes all whitespace and the user may end up including redundant whitespace. This is hacky and a faulty assumption given that we allow custom #Layout.

For now, we just remove the trim() call. This shouldn't cause issues downstream - I've manually checked that all List{...} and NeList{...} declarations in the EVM, C, and WASM semantics are already correct. If anything was missed, it's an easy fix.

A proper solution is described in #4595

@Scott-Guest Scott-Guest self-assigned this Aug 20, 2024
@Scott-Guest Scott-Guest marked this pull request as ready for review August 20, 2024 02:38
@ehildenb
Copy link
Member

For now, let's add a warning that is printed if you have a separator that is only whitespace. I guess that check can be implemented manually, just check against regex "[ \n\r\t]*"

@rv-jenkins rv-jenkins merged commit 0055fe7 into develop Aug 22, 2024
17 checks passed
@rv-jenkins rv-jenkins deleted the no-trim branch August 22, 2024 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants