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

Support CRLF ("\r\n") as a valid line separator in scripts #918

Closed
SitiSchu opened this issue Feb 18, 2020 · 6 comments
Closed

Support CRLF ("\r\n") as a valid line separator in scripts #918

SitiSchu opened this issue Feb 18, 2020 · 6 comments
Labels
Milestone

Comments

@SitiSchu
Copy link
Contributor

@SitiSchu SitiSchu commented Feb 18, 2020

Supporting CRLF (\r\n) line endings would fix issues with modules installed using epm not working on windows due to git's autocrlf setting.

@SitiSchu SitiSchu changed the title Support CRLF in .elv files Support CRLF in elvish Feb 24, 2020
@SitiSchu
Copy link
Contributor Author

@SitiSchu SitiSchu commented Feb 24, 2020

I changed the title to be more generic since things like curl -I return output containg CRLF so when capturing the output and trying to put it into a list the list items will have \r in them

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 26, 2020

Does this proposal apply only to elvish scripts? Or does it also apply to the implicit line splitting done by the (...) output capture and the implicit splitting done by many commands such as this example:

~> echo "abc\ndef" | each $str:to-upper~
▶ ABC
▶ DEF

How would this affect read-upto? Should this only be done on platforms (e.g., MS Windows) where CRLF line endings are native or also on UNIX like platforms?

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Feb 26, 2020

Regarding how this would interact with read-upto see #907 which discusses introducing a dedicated read-line function.

@SitiSchu
Copy link
Contributor Author

@SitiSchu SitiSchu commented Feb 26, 2020

It was only for scripts before but then I changed it to apply it to the implicit splitting too (since headers in http use CRLF)

@xiaq xiaq changed the title Support CRLF in elvish Support CRLF in elvish scripts Apr 17, 2020
@xiaq
Copy link
Member

@xiaq xiaq commented Apr 17, 2020

I changed the title back to supporting CRLF in elvish scripts - I don't see the harm in supporting this.

Supporting CRLF in the implicit splitting of output capture is a different feature and is tracked separately in #970.

@xiaq xiaq changed the title Support CRLF in elvish scripts Support CRLF ("\r\n") as a valid line separator in scripts Apr 17, 2020
@xiaq xiaq added the A:Language label Apr 17, 2020
@xiaq xiaq added this to the 0.14 milestone Apr 17, 2020
@xiaq
Copy link
Member

@xiaq xiaq commented May 3, 2020

Related and interesting: Go supports using \r\n as line endings in sources, but strips raw \r from multi-line string literals.

The thinking seems to be that converting the line endings of a source file shouldn't change it from the Go compiler's perspective. This is a useful property, but I am not sure Go's approach is a good way to implement it, so I'll default to not adopting the stripping behavior.

@xiaq xiaq closed this in 06901fb May 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.