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

Add read-line command #975

Closed
xiaq opened this issue Apr 19, 2020 · 4 comments
Closed

Add read-line command #975

xiaq opened this issue Apr 19, 2020 · 4 comments
Milestone

Comments

@xiaq
Copy link
Member

@xiaq xiaq commented Apr 19, 2020

Currently read-line can be implemented in terms of read-upto, but it would be nice to have a dedicated read-line command too.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Apr 20, 2020

Let me start by saying I haven't used MS Windows as a developer in more than two decades. So perhaps the rest of my comment is ill-informed regarding best practices for splitting text on line boundaries on MS Windows. In fact, I've found a few articles that talk about this. Such as https://www.theregister.co.uk/2018/05/08/windows_notepad_unix_linux_macos/ from two years ago which notes that Windows Notepad application finally does something reasonable when opening a file with UNIX line endings.

The problem with implementing read-line as a function built in terms of read-upto is that it seems impossible to make it work correctly on UNIX and MS Windows. Some of the difficulty in doing so was recently ameliorated by the introduction of the $platform:is-unix and $platform:is-windows variables. Those can be used to ignore \r on UNIX platforms.

The difficulty is how to handle the \r\n sequence with full fidelity on MS Windows if either char of the pair is missing. You can't just read-upto a \n then strip the \r, if any, that is in the returned string AFAIK to be strictly conforming with the MS Windows definition of a line. Perhaps being strictly conforming with old versions of MS Windows (and MS DOS) is no longer appropriate. And splitting on any of the chars individually, and the \r\n pair, is now acceptable and/or a best practice.

@krader1961
Copy link
Contributor

@krader1961 krader1961 commented Apr 20, 2020

I am inclined to recommend that Elvish implement a read-line that accepts \r, \n or \r\n. Furthermore, that implicit splitting of command byte output be done using the same criteria. Regardless of the platform. Where more control is needed the read-upto command should be used. Although that represents difficulties since if you do x = (read-upto "\n") you presumably don't want any trailing \r that remains to be removed if you're on a Windows platform. Or are processing output from a program, or standard, where \r\n is the end-of-line marker.

It seems like this ultimately boils down to how easily we want Elvish to be able to process arbitrary binary data without using mechanisms like the slurp command (which might also need to be special-cased).

@xiaq
Copy link
Member Author

@xiaq xiaq commented May 6, 2020

It is not possible to implement a command that accepts \r and \r\n at the same time (it wouldn't know whether to continue going after \r). None of the platforms supported by Elvish uses \r, so read-line will just support \n or \r\n.

@xiaq xiaq added this to the 0.14 milestone May 6, 2020
@xiaq
Copy link
Member Author

@xiaq xiaq commented May 6, 2020

Adding this to 0.14 since read-line can replace use of take 1 for reading a single line from byte input, and #923 is in 0.14.

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
2 participants
You can’t perform that action at this time.