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

Normalize newlines before passing to scripts #249

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

WardBrian
Copy link
Collaborator

Reported here: https://discourse.mc-stan.org/t/stan-playground-issues-with-pasting-scripts/37498

WebR complains about invalid tokens if given windows CRLF line endings. The easiest way to test this on non-windows is save a file in your editor using these endings, and then use the load project window to upload it (copy-pasting seems to remove them, on my Ubuntu machine at least)

I looked through the Monaco docs to see if there was a way we could ask it to do this normalization for us, but it seems there isn't, so I just put it right before we actually shell out to webr or pyodide (pyodide doesn't seem to mind either way)

@WardBrian WardBrian requested a review from jsoules December 4, 2024 14:58
@magland
Copy link
Collaborator

magland commented Dec 4, 2024

Looks good to me.

Copy link
Collaborator

@jsoules jsoules left a comment

Choose a reason for hiding this comment

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

This seems fine. I can't think of a case where you actually need to persist a carriage return for anything, and I'm okay saying we just don't support that if we do find one.

@jsoules
Copy link
Collaborator

jsoules commented Dec 4, 2024

Addresses #250

@WardBrian WardBrian linked an issue Dec 4, 2024 that may be closed by this pull request
@WardBrian WardBrian merged commit 75d50f9 into main Dec 4, 2024
2 checks passed
@WardBrian WardBrian deleted the scripts-normalize-line-endings branch December 4, 2024 16:32
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.

Issue with carriage return/line feed
3 participants