preserve line endings (Fix issue #24) #26
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR fixes issue #24 by ensuring the original line endings (LF/CRLF/CR) are preserved when processing and writing pre-commit config files. It updates the core text‐processing logic to retain end-of-line characters, modifies the CLI file reading mechanism to avoid newline translation, adds targeted tests for all common line endings in both processing and CLI execution, and updates the changelog accordingly. Sequence diagram for pre-commit config file processing with preserved line endingssequenceDiagram
participant CLI
participant FileSystem
participant "process_precommit_text()"
CLI->>FileSystem: Read pre-commit config file as bytes
FileSystem-->>CLI: Return file contents (with original line endings)
CLI->>"process_precommit_text()": Pass file contents
"process_precommit_text()"->>"process_precommit_text()": Split lines (preserving line endings)
"process_precommit_text()"-->>CLI: Return processed text (with original line endings)
CLI->>FileSystem: Write processed config file (preserving line endings)
Class diagram for updated pre-commit config processing functionsclassDiagram
class CLI {
+process_precommit()
-precommit_filename.read_bytes()
}
class process_precommit_text {
+process_precommit_text(precommit_text, uv_data, user_repo_mappings, user_version_mappings)
-repo_header_re
-repo_rev_re
-lines: splitlines(keepends=True)
-new_lines: list[str]
+returns: (str, dict)
}
CLI --> process_precommit_text
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #26 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 149 149
Branches 29 29
=========================================
Hits 149 149 ☔ View full report in Codecov by Sentry. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The docstring for test_process_precommit_text_preserves_line_endings doesn’t match its behavior—it tests a version bump, not ‘no version change’, so please update it for clarity.
- In cli.py, consider writing the fixed pre-commit content in binary mode or with write_text(newline='') so the original line endings are guaranteed preserved when saving the file.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The docstring for test_process_precommit_text_preserves_line_endings doesn’t match its behavior—it tests a version bump, not ‘no version change’, so please update it for clarity.
- In cli.py, consider writing the fixed pre-commit content in binary mode or with write_text(newline='') so the original line endings are guaranteed preserved when saving the file.
## Individual Comments
### Comment 1
<location> `tests/test_sync.py:311-334` </location>
<code_context>
+ ["\n", "\r\n", "\r"],
+ ids=["LF", "CRLF", "CR"],
+)
+def test_process_precommit_text_preserves_line_endings_no_version_change(
+ line_ending: str,
+) -> None:
+ """Test that line endings are preserved when version is already correct."""
+ precommit_text = line_ending.join(
+ [
+ "repos:",
+ "- repo: https://github.com/psf/black-pre-commit-mirror",
+ " rev: 23.11.0",
+ " hooks:",
+ " - id: black",
+ "- repo: https://github.com/unchanged/unchanged",
+ " rev: 1.2.3",
+ " hooks:",
+ " - id: unchanged",
+ ]
+ )
+ # Package exists in uv.lock but version is already correct
+ uv_data = {"black": "23.11.0"}
+
+ result, _changes = process_precommit_text(precommit_text, uv_data)
+
+ # Result should be identical to input when version is already correct
+ assert result == precommit_text
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test case for mixed line endings within the same file.
Testing files with both LF and CRLF line endings will help ensure your logic handles mixed scenarios correctly.
```suggestion
def test_process_precommit_text_preserves_line_endings_no_version_change(
line_ending: str,
) -> None:
"""Test that line endings are preserved when version is already correct."""
precommit_text = line_ending.join(
[
"repos:",
"- repo: https://github.com/psf/black-pre-commit-mirror",
" rev: 23.11.0",
" hooks:",
" - id: black",
"- repo: https://github.com/unchanged/unchanged",
" rev: 1.2.3",
" hooks:",
" - id: unchanged",
]
)
# Package exists in uv.lock but version is already correct
uv_data = {"black": "23.11.0"}
result, _changes = process_precommit_text(precommit_text, uv_data)
# Result should be identical to input when version is already correct
assert result == precommit_text
def test_process_precommit_text_preserves_mixed_line_endings_no_version_change() -> None:
"""Test that mixed line endings are preserved when version is already correct."""
# Construct a precommit_text with mixed line endings (LF and CRLF)
precommit_text = (
"repos:\n"
"- repo: https://github.com/psf/black-pre-commit-mirror\r\n"
" rev: 23.11.0\n"
" hooks:\r\n"
" - id: black\n"
"- repo: https://github.com/unchanged/unchanged\r\n"
" rev: 1.2.3\n"
" hooks:\r\n"
" - id: unchanged\n"
)
uv_data = {"black": "23.11.0"}
result, _changes = process_precommit_text(precommit_text, uv_data)
# Result should be identical to input when version is already correct
assert result == precommit_text
```
</issue_to_address>
### Comment 2
<location> `tests/test_sync.py:314` </location>
<code_context>
+ ["\n", "\r\n", "\r"],
+ ids=["LF", "CRLF", "CR"],
+)
+def test_process_precommit_text_preserves_line_endings(
+ line_ending: str,
+) -> None:
+ """Test that line endings are preserved when version is already correct."""
+ precommit_text = line_ending.join(
+ [
+ "repos:",
+ "- repo: https://github.com/psf/black-pre-commit-mirror",
+ " rev: 23.11.0",
+ " hooks:",
+ " - id: black",
+ "- repo: https://github.com/unchanged/unchanged",
+ " rev: 1.2.3",
+ " hooks:",
+ " - id: unchanged",
+ ]
+ )
+ # Package exists in uv.lock but version is already correct
+ uv_data = {"black": "24.0.0"}
+
+ result, _changes = process_precommit_text(precommit_text, uv_data)
+
+ # Result should be identical to input when version is already correct
+ assert result == precommit_text.replace("23.11.0", "24.0.0")
</code_context>
<issue_to_address>
**suggestion (testing):** Test description is misleading: version is changed, not just line endings preserved.
Please revise the docstring to indicate that the test updates the version, not just preserves line endings.
```suggestion
"""Test that line endings are preserved and the version is updated in the output."""
```
</issue_to_address>
### Comment 3
<location> `tests/test_cli.py:367-376` </location>
<code_context>
+def test_cli_preserves_line_endings_when_writing(
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for the scenario where no version change is needed.
Please add a test to verify that when the version is already correct, the function preserves line endings without making changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+311
to
+334
| def test_process_precommit_text_preserves_line_endings_no_version_change( | ||
| line_ending: str, | ||
| ) -> None: | ||
| """Test that line endings are preserved when version is already correct.""" | ||
| precommit_text = line_ending.join( | ||
| [ | ||
| "repos:", | ||
| "- repo: https://github.com/psf/black-pre-commit-mirror", | ||
| " rev: 23.11.0", | ||
| " hooks:", | ||
| " - id: black", | ||
| "- repo: https://github.com/unchanged/unchanged", | ||
| " rev: 1.2.3", | ||
| " hooks:", | ||
| " - id: unchanged", | ||
| ] | ||
| ) | ||
| # Package exists in uv.lock but version is already correct | ||
| uv_data = {"black": "23.11.0"} | ||
|
|
||
| result, _changes = process_precommit_text(precommit_text, uv_data) | ||
|
|
||
| # Result should be identical to input when version is already correct | ||
| assert result == precommit_text |
There was a problem hiding this comment.
suggestion (testing): Consider adding a test case for mixed line endings within the same file.
Testing files with both LF and CRLF line endings will help ensure your logic handles mixed scenarios correctly.
Suggested change
| def test_process_precommit_text_preserves_line_endings_no_version_change( | |
| line_ending: str, | |
| ) -> None: | |
| """Test that line endings are preserved when version is already correct.""" | |
| precommit_text = line_ending.join( | |
| [ | |
| "repos:", | |
| "- repo: https://github.com/psf/black-pre-commit-mirror", | |
| " rev: 23.11.0", | |
| " hooks:", | |
| " - id: black", | |
| "- repo: https://github.com/unchanged/unchanged", | |
| " rev: 1.2.3", | |
| " hooks:", | |
| " - id: unchanged", | |
| ] | |
| ) | |
| # Package exists in uv.lock but version is already correct | |
| uv_data = {"black": "23.11.0"} | |
| result, _changes = process_precommit_text(precommit_text, uv_data) | |
| # Result should be identical to input when version is already correct | |
| assert result == precommit_text | |
| def test_process_precommit_text_preserves_line_endings_no_version_change( | |
| line_ending: str, | |
| ) -> None: | |
| """Test that line endings are preserved when version is already correct.""" | |
| precommit_text = line_ending.join( | |
| [ | |
| "repos:", | |
| "- repo: https://github.com/psf/black-pre-commit-mirror", | |
| " rev: 23.11.0", | |
| " hooks:", | |
| " - id: black", | |
| "- repo: https://github.com/unchanged/unchanged", | |
| " rev: 1.2.3", | |
| " hooks:", | |
| " - id: unchanged", | |
| ] | |
| ) | |
| # Package exists in uv.lock but version is already correct | |
| uv_data = {"black": "23.11.0"} | |
| result, _changes = process_precommit_text(precommit_text, uv_data) | |
| # Result should be identical to input when version is already correct | |
| assert result == precommit_text | |
| def test_process_precommit_text_preserves_mixed_line_endings_no_version_change() -> None: | |
| """Test that mixed line endings are preserved when version is already correct.""" | |
| # Construct a precommit_text with mixed line endings (LF and CRLF) | |
| precommit_text = ( | |
| "repos:\n" | |
| "- repo: https://github.com/psf/black-pre-commit-mirror\r\n" | |
| " rev: 23.11.0\n" | |
| " hooks:\r\n" | |
| " - id: black\n" | |
| "- repo: https://github.com/unchanged/unchanged\r\n" | |
| " rev: 1.2.3\n" | |
| " hooks:\r\n" | |
| " - id: unchanged\n" | |
| ) | |
| uv_data = {"black": "23.11.0"} | |
| result, _changes = process_precommit_text(precommit_text, uv_data) | |
| # Result should be identical to input when version is already correct | |
| assert result == precommit_text |
tsvikas
force-pushed
the
fix/issue-24
branch
2 times, most recently
from
5c6bbd6 to
94cac6b
Compare
Fix handling of line endings in .pre-commit-config.yaml by reading the
file as bytes and using splitlines(keepends=True) to maintain original
line ending style (CRLF vs LF). This ensures the output file preserves
the same line ending format as the input.
Previously, the code used read_text() and split("\n"), which would
normalize all line endings to LF and potentially corrupt files that
used CRLF line endings on Windows.
Fixes #24
Fix handling of line endings in .pre-commit-config.yaml by reading the
file as bytes and using splitlines(keepends=True) to maintain original
line ending style (CRLF vs LF). This ensures the output file preserves
the same line ending format as the input.
Previously, the code used read_text() and split("\n"), which would
normalize all line endings to LF and potentially corrupt files that
used CRLF line endings on Windows.
Fixes #24
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Preserve original line endings when reading, processing, and writing pre-commit configuration to avoid altering line ending formats.
Bug Fixes:
Enhancements:
Documentation:
Tests: