-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add String.lines/1 #14493
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
base: main
Are you sure you want to change the base?
Add String.lines/1 #14493
Conversation
defp lines(<<?\n, rest::binary>>, acc), | ||
do: [<<acc::binary, ?\n>> | lines(rest, <<>>)] | ||
|
||
defp lines(<<char, rest::binary>>, acc), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we handle /r
as well? the python version does:
"asd\radf".splitlines(True)
['asd\r', 'adf']
Side note: LSP document synchronisation treats /r
as legit newlines as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Ruby does one not and I am skeptical about doing so because it is clearly not a newline on terminals:
$ iex
Erlang/OTP 27 [erts-15.1.2] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit]
Interactive Elixir (1.19.0-dev) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> IO.puts "foo\rbar"
bar
:ok
So treating it as a newline looks very Windows centric. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps support the Unicode Line Separator? That would seem consistent with String.split/1
which uses the Unicode definition of whitespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So treating it as a newline looks very Windows centric. :)
Wasn't it classic MacOS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps support the Unicode Line Separator? That would seem consistent with
String.split/1
which uses the Unicode definition of whitespace.
python does support it
"asd
adf".splitlines(True)
['asd\u2028', 'adf']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you point, which is why I can totally understand not including VT and FF (haven't used those since punch cards and line printers!!!!). And PS I suppose.
A bit sad that Zed doesn't interpret \u2028
though and perhaps that's worth raising as an issue there (which I'm happy to do). I don't think that's noted in the Unicode security guide but I'll check that first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python has it's own rules https://docs.python.org/3/library/stdtypes.html#str.splitlines basing on https://peps.python.org/pep-0278/ and https://peps.python.org/pep-3116/
And they quite recently added \v and \f to list of line boundaries (in 3.2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And then you have Erlang's interpretation of newlines, which only considers \r\n and \n as well:
~/OSS/elixir[jv-string-lines *%]$ cat example | elixir -e "IO.stream() |> Enum.each(&IO.inspect/1)"
"LINE\n"
"LINE\rLINE\n"
<<76, 73, 78, 69, 12, 76, 73, 78, 69, 11, 76, 73, 78, 69, 194, 133, 76, 73, 78,
69, 226, 128, 168, 76, 73, 78, 69, 226, 128, 169, 76, 73, 78, 69>>
So I am thinking the best is to find a new home for this function. Perhaps the Code
module indeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's even more conservative as it only shows \n
but I assume there is a configuration somewhere for it to read it Windows style.
Inspired by
String#lines
in Ruby andstring.splitlines(True)
in Python.