-
Notifications
You must be signed in to change notification settings - Fork 302
Description
Currently, the check() function of rules/new_lines.py is only called once per file, checking only the first new-line-character of a file. (see also this comment)
To also be able to detect mixed newlines (and report the wrong ones) all lines of a file should be checked.
Example
Consider this yaml-file (with the newline characters explicitly shown)
---\n
test\r\n
\nIf it is checked for UNIX-style newlines, it should report an error on line 2 and if it is checked for DOS-style newlines it should report two errors on lines 1 and 3.
Currently, this file would be valid if checked with type: unix and invalid with type: dos (with only one error on line 1).
Discussion
It should be discussed if all wrong newline characters should be reported.
In my opinion, this would clutter the error output too much and the amount of errors resulting from this should be caped. Either only one new-lines error per file or only a small amount (maybe max of 5?). Then the error message should say something like "more new-line errors possible, but not showing..."
side effects
PR #474 introduced a new type and tests for it. Some test where supposed to test this type of scenario and therefore are currently broken. They can be reactivated once this issue is fixed. See:
yamllint/tests/rules/test_new_lines.py
Lines 77 to 82 in 40cab7f
| # FIXME: the following tests currently don't work | |
| # because only the first line is checked for line-endings | |
| # see: issue #475 | |
| # --- | |
| # self.check('---\ntext\r\nfoo\n', conf, problem=(2, 4)) | |
| # self.check('---\ntext\r\n', conf, problem=(2, 4)) |
yamllint/tests/rules/test_new_lines.py
Lines 91 to 96 in 40cab7f
| # FIXME: the following tests currently don't work | |
| # because only the first line is checked for line-endings | |
| # see: issue #475 | |
| # --- | |
| # self.check('---\r\ntext\nfoo\r\n', conf, problem=(2, 4)) | |
| # self.check('---\r\ntext\n', conf, problem=(2, 4)) |