-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
#177: Forbid to use CRLF end-of-line #708
base: master
Are you sure you want to change the base?
#177: Forbid to use CRLF end-of-line #708
Conversation
Hey @sobolevn . Should file |
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.
Thanks!
dotenv_linter/violations/values.py
Outdated
Restricts to use `/r/n` (CRLF) end-of-line. | ||
Reasoning: | ||
??? |
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.
Mixing different end-of-line chars can lead to different hard-to-debug problems.
dotenv_linter/violations/values.py
Outdated
??? | ||
Solution: | ||
Use `/n` (LF) end-of-line. |
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.
Please, mention .gitattributes
file here.
dotenv_linter/violations/values.py
Outdated
Solution: | ||
Use `/n` (LF) end-of-line. | ||
Example:: |
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.
Please, remove this section.
dotenv_linter/visitors/fst/values.py
Outdated
@@ -35,3 +39,9 @@ def _check_value_quotes(self, node: Value) -> None: | |||
self._add_violation(QuotedValueViolation(node, text=node.raw_text)) | |||
elif text.startswith("'") and text.endswith("'"): | |||
self._add_violation(QuotedValueViolation(node, text=node.raw_text)) | |||
|
|||
def _is_crlf_eol_used(self, node: Value) -> None: | |||
crlf_eol = '\r' |
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.
Please, make this a module-level final constant.
setup.cfg
Outdated
@@ -65,7 +65,8 @@ addopts = | |||
--strict-markers | |||
--strict-config | |||
--doctest-modules | |||
--cov=dotenv_linter | |||
# Disable with option to allow debugging | |||
; --cov=dotenv_linter |
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.
Please, re-enable
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.
No problem. Should it be mentioned somewhere? For example, in Development
README section?
tests/fixtures/.env.correct
Outdated
@@ -4,3 +4,4 @@ | |||
# See: https://github.com/wemake-services/dotenv-linter/issues/20 | |||
TEST=1 | |||
EMPTY= | |||
VARIABLE_WITH_TRAILING_SLASH_R=value/r |
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.
/r
and \r
are clearly not the same
tests/fixtures/.env.eol
Outdated
@@ -0,0 +1 @@ | |||
VARIABLE_WITH_CRLF_EOL=123 |
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.
Please, make this test a code-only, no fixtures should be used. Since there are lots of editors / tools / etc that fix eofs.
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.
Just to make it clear - Are you suggest creating temp file inside test?
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.
yes
tests/test_cli/test_lint_command.py
Outdated
assert str(violation_class.code) in stderr | ||
|
||
|
||
def test_lint_wrong_eol(fixture_path: Callable) -> None: |
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.
def test_lint_wrong_eol(fixture_path: Callable) -> None: | |
def test_lint_wrong_eol(fixture_path: Callable[[str], str]) -> None: |
Is violation name suitable? |
I have made things!
Checklist
CHANGELOG.md
Related issues
\r\n
for line breaks #177