Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions spec/std/string_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2700,8 +2700,26 @@ describe "String" do
lines.should eq(["foo\n", "\n", "bar\r\n", "baz\r\n"])
end

it "gets each_line with remove_empty = true" do
lines = [] of String
"\nfoo\n\nbar\r\nbaz\n\n".each_line(remove_empty: true) do |line|
lines << line
end.should be_nil
lines.should eq(["foo", "bar", "baz"])
end

it "gets each_line with remove_empty = true and chomp = false" do
lines = [] of String
"\nfoo\n\nbar\r\n\r\nbaz".each_line(remove_empty: true, chomp: false) do |line|
lines << line
end.should be_nil
lines.should eq(["foo\n", "bar\r\n", "baz"])
end

it_iterates "#each_line", ["foo", "bar", "baz"], "foo\nbar\r\nbaz\r\n".each_line
it_iterates "#each_line(chomp: false)", ["foo\n", "bar\r\n", "baz\r\n"], "foo\nbar\r\nbaz\r\n".each_line(chomp: false)
it_iterates "#each_line(remove_empty: true)", ["foo", "bar", "baz"], "\nfoo\n\nbar\r\n\r\nbaz".each_line(remove_empty: true)
it_iterates "#each_line(remove_empty: true, chomp: false)", ["foo\n", "bar\r\n", "baz"], "\nfoo\n\nbar\r\n\r\nbaz".each_line(remove_empty: true, chomp: false)

it_iterates "#each_codepoint", [97, 98, 9731], "ab☃".each_codepoint

Expand Down
37 changes: 32 additions & 5 deletions src/string.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4395,6 +4395,8 @@ class String
# "hello\nworld\r\n".each_line(chomp: false) { } # yields "hello\n", "world\r\n"
# ```
#
# If *remove_empty* is `true`, any empty strings are removed from the result.
#
# A trailing line feed is not considered starting a final, empty line. The
# empty string does not contain any lines.
#
Expand All @@ -4405,21 +4407,33 @@ class String
# ```
#
# * `#lines` returns an array of lines
def each_line(chomp : Bool = true, & : String ->) : Nil
def each_line(chomp : Bool = true, remove_empty : Bool = false, & : String ->) : Nil
return if empty?

offset = 0

while byte_index = byte_index('\n'.ord.to_u8, offset)
count = byte_index - offset + 1

if remove_empty
line_length = count - 1
if offset + line_length > 0 && to_unsafe[offset + line_length - 1] === '\r'
line_length -= 1
end

skip_line = line_length == 0
else
skip_line = false
end
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: This logic seems a bit convoluted.
The empty line detection can be simplified to byte_index == offset || (byte_index == offset + 1 && to_unsafe[byte_index - 1] === '\r').

Also, the loop might be a bit better readable if directly skip to the next iteration with next.

Suggested change
if remove_empty
line_length = count - 1
if offset + line_length > 0 && to_unsafe[offset + line_length - 1] === '\r'
line_length -= 1
end
skip_line = line_length == 0
else
skip_line = false
end
if remove_empty && (byte_index == offset || (byte_index == offset + 1 && to_unsafe[byte_index - 1] === '\r'))
offset = byte_index + 1
next
end

Ditto for the iterator. There we can return early with self.next.

Copy link
Contributor Author

@andrykonchin andrykonchin Oct 21, 2025

Choose a reason for hiding this comment

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

Thank you! Done.


if chomp
count -= 1
if offset + count > 0 && to_unsafe[offset + count - 1] === '\r'
count -= 1
end
end

yield unsafe_byte_slice_string(offset, count)
yield unsafe_byte_slice_string(offset, count) unless skip_line
offset = byte_index + 1
end

Expand All @@ -4429,8 +4443,8 @@ class String
end

# Returns an `Iterator` which yields each line of this string (see `String#each_line`).
def each_line(chomp = true)
LineIterator.new(self, chomp)
def each_line(chomp = true, *, remove_empty : Bool = false)
LineIterator.new(self, chomp, remove_empty)
end

# Converts camelcase boundaries to underscores.
Expand Down Expand Up @@ -5699,17 +5713,29 @@ class String
private class LineIterator
include Iterator(String)

def initialize(@string : String, @chomp : Bool)
def initialize(@string : String, @chomp : Bool, @remove_empty : Bool)
@offset = 0
@end = false
end

def next
return stop if @end

skip_line = false

byte_index = @string.byte_index('\n'.ord.to_u8, @offset)
if byte_index
count = byte_index - @offset + 1

if @remove_empty
line_length = count - 1
if @offset + line_length > 0 && @string.to_unsafe[@offset + line_length - 1] === '\r'
line_length -= 1
end

skip_line = line_length == 0
end

if @chomp
count -= 1
if @offset + count > 0 && @string.to_unsafe[@offset + count - 1] === '\r'
Expand All @@ -5728,6 +5754,7 @@ class String
@end = true
end

value = self.next if skip_line
value
end
end
Expand Down
Loading