Remove empty lines from String#each_line

[andrykonchin]

Changes:

• added remove_empty parameter for method def each_line(chomp : Bool = true, & : String ->) : Nil
• added remove_empty parameter for method def each_line(chomp = true)

Closes #15149

[andrykonchin]

I am concerned about handling "\n" lines when given chomp: false, so a line is logically "empty" but a String object returned to a user is not.

[ysbaddaden]

The line itself is still empty?

[straight-shoota]

remove_empty applies a filter, chomp defines how the items are presented.
The empty line filter should not be affected by whether non-empty lines end with a NL character or not.

[andrykonchin]

The line itself is still empty?

Confusion that may happen is what is filtered with the remove_empty filter - lines or strings as far as "\n" is a non-empty string but an empty line.

It seems logical to apply the filter to lines, that's a "\n" line will be filtered out regardless of the chomp option value. Does is make sense?

[straight-shoota]

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.

[andrykonchin]

Thank you! Done.