Replace string replacement with HTML parser in render method#361
Replace string replacement with HTML parser in render method#361
Conversation
Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com>
…tion Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## safari #361 +/- ##
==========================================
- Coverage 98.20% 97.72% -0.48%
==========================================
Files 8 8
Lines 223 264 +41
==========================================
+ Hits 219 258 +39
- Misses 4 6 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com>
I've compressed the parser code from ~60 lines to ~35 lines by:
Added comprehensive unit tests:
All 46 tests pass with 99% coverage on forms.py. Changes committed in f1fea21. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the render method in S3FileInputMixin to use an HTML parser instead of fragile string replacement for transforming <input type="file"> elements to <s3-file> custom elements. This makes the transformation more robust to Django rendering changes and variations in attribute ordering.
Key changes:
- Introduced
InputToS3FileRewriterclass that extends Python'sHTMLParserto parse and transform HTML - Updated the
rendermethod to use the new parser instead of string replacement - Added 10 comprehensive unit tests covering parser functionality and render method behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| s3file/forms.py | Added InputToS3FileRewriter HTML parser class and updated render method to use it for transforming file inputs to custom elements |
| tests/test_forms.py | Added 7 tests for the parser class and 3 tests for the render method to verify correct transformation, attribute handling, and HTML escaping |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
for more information, see https://pre-commit.ci
|
@copilot make sure to add tests for all handlers |
Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com>
Co-authored-by: codingjoe <1772890+codingjoe@users.noreply.github.com>
Added tests for all remaining handlers:
All 13 parser tests now pass with 98% coverage on forms.py. Committed in e736b37. |
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.