Do not report CRLF sequences in Layout/TrailingWhitespace rule#627
Do not report CRLF sequences in Layout/TrailingWhitespace rule#627
Layout/TrailingWhitespace rule#627Conversation
| # source.lines # => ["a = 1", "b = 2"] | ||
| # ``` | ||
| getter lines : Array(String) { code.split('\n') } | ||
| getter lines : Array(String) { code.split(/\r?\n/) } |
There was a problem hiding this comment.
Any reason not to use String#lines? Should be much more efficient.
There was a problem hiding this comment.
Yes, String#lines eats the final newline:
str = <<-TEXT
foo
bar\t
TEXT
str.lines # => ["foo ", "bar\t", ""]
str.split(/\r?\n/) # => ["foo ", "bar\t", "", ""]There was a problem hiding this comment.
Is that really relevant, though?
I believe this should only matter for Lint/TrailingBlankLines which could do a simpler, string-based test.
There was a problem hiding this comment.
(\n) lines 5.58k (179.32µs) (± 5.45%) 527kB/op fastest
(\n) regex split 2.12k (471.77µs) (± 4.67%) 729kB/op 2.63× slower
(\n) gsub + split 2.28k (438.07µs) (± 3.47%) 527kB/op 2.44× slower
(\r\n) lines 5.47k (182.86µs) (± 2.04%) 527kB/op 1.02× slower
(\r\n) regex split 2.05k (487.75µs) (± 0.91%) 729kB/op 2.72× slower
(\r\n) gsub + split 1.74k (573.81µs) (± 1.94%) 721kB/op 3.20× slower
There was a problem hiding this comment.
Source#lines is expected to return accurate representation of the underlying data, without any modifications, so I'd prefer to stick to correctness over efficiency here.
On a related note, why the String#lines is eating the final newline? It seems to me that's either a bug, or an undocumented behavior at least.
There was a problem hiding this comment.
I think both interpretations are valid. There's no clear answer whether \n is one or two lines. We have to live with the ambiguity.
The reasoning for one is: The line feed character \n marks the end of a line. That does not necessarily imply the start of a new line. At the end of the file/string there's no content afterwards. So it's reasonable to not counting a trailing newline as an additional empty line.
The documentation is certainly sparse on this, though. String#lines has no docs whatsoever. String#each_line says it "splits the string after each newline", which matches the observed behaviour.
There was a problem hiding this comment.
I agree. Having a keep_trailing_newline = false named parameter added to String#lines would help to achieve both behaviors.
4 participants
Resolves #622