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.

Sign up for GitHub

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

Jump to bottom

Allow CRLF at the start of multiline strings #255

Merged
merged 3 commits into from
Aug 9, 2024
Merged

Allow CRLF at the start of multiline strings #255

merged 3 commits into from
Aug 9, 2024

Conversation

mbartlett21
Copy link
Contributor

@mbartlett21 mbartlett21 commented Aug 2, 2024 •
edited
Loading

This should fix #254.

I also fixed another bug while I was at it. Gren was silently dropping the last newline if the string didn't end with a newline.

robinheghan
robinheghan requested changes Aug 3, 2024
View reviewed changes
Copy link
Member

@robinheghan robinheghan left a comment

Choose a reason for hiding this comment

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

Good first attempt, but I think we can do better.

Also, please add your name to the CONTRIBUTORS file, if it isn't there already.

compiler/src/Parse/String.hs Outdated Show resolved Hide resolved
compiler/src/Reporting/Error/Syntax.hs Show resolved Hide resolved
tests/Parse/MultilineStringSpec.hs Show resolved Hide resolved
@mbartlett21
Allow multiline strings to work properly with \r\n
4961a54 
They are always encoded in the end result as just \n
@robinheghan
Copy link
Member

Code formatting check fails.

To apply correct code formatting, install ormolu and run ormolu --mode inplace $(git ls-files '*.hs')

@mbartlett21
Copy link
Contributor Author

@robinheghan
I've added another test for if it doesn't end with a newline, which was previously wrong.
I.e.

"""testing
testing"""

previously dropped the newline

robinheghan
robinheghan approved these changes Aug 9, 2024
View reviewed changes
Copy link
Member

@robinheghan robinheghan left a comment

Choose a reason for hiding this comment

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

Thank you for this <3

@robinheghan robinheghan merged commit 42ce9c0 into gren-lang:main Aug 9, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robinheghan robinheghan robinheghan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Multi-line strings don't work on Windows
2 participants
@mbartlett21 @robinheghan