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

Stop using /**/ to signal blank lines #632

Merged
merged 2 commits into from
Sep 10, 2024
Merged

Stop using /**/ to signal blank lines #632

merged 2 commits into from
Sep 10, 2024

Conversation

hugomg
Copy link
Member

@hugomg hugomg commented Sep 6, 2024

In-band signaling is evil :)

The original idea behind /**/ was that it was difficult to control when the coder generated a blank line, so we told the reindenter to delete all blank lines unless they were specially marked with /**/. However, it turns out that it's not too difficult to avoid unwanted blank lines. The main problem used to be situations where we did a table.concat("\n") where some elements in the list already have a newline. The fix is the concat_lines function, which removes said newlines before we concat.

In-band signaling is evil :)

The original idea behind /**/ was that it was difficult to control when
the coder generated a blank line, so we told the reindenter to delete
all blank lines unless they were specially marked with /**/. However,
it turns out that it's not too difficult to avoid unwanted blank lines.
The main problem used to be situations where we did a table.concat("\n")
where some elements in the list already have a newline. The fix is the
`concat_lines` function, which removes said newlines before we concat.
- Pay attention to blank lines
- Check USHRT_MAX at Pallene compile-time instead of C compile-time.
@hugomg hugomg merged commit 6de8cdb into master Sep 10, 2024
2 checks passed
@hugomg hugomg deleted the reformat-newlines branch September 10, 2024 03:31
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.

1 participant