Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
==========================================
+ Coverage 81.22% 81.47% +0.25%
==========================================
Files 94 94
Lines 6646 6760 +114
==========================================
+ Hits 5398 5508 +110
- Misses 1248 1252 +4
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:
|
|
|
||
| def _split_css_declarations(style: str) -> list[str]: | ||
| declarations = [] | ||
| current = [] |
There was a problem hiding this comment.
Any reason not to implement current as an empty string that gets progressively modified rather than an empty list that gets converted later?
There was a problem hiding this comment.
technically more efficient (probably) as avoids string reallocations, but no particularly compelling reason - i think either way is clear
| current.append(char) | ||
| continue | ||
| if char == ")" and paren_depth > 0: | ||
| paren_depth -= 1 |
There was a problem hiding this comment.
Should this have error handling for invalid CSS declarations? Parentheses depth less than 0 should probably throw a template syntax error.
There was a problem hiding this comment.
OK, having looked at the tests it seems the standard is to ignore invalid declarations. I'm not sure what unexpected behaviour might arise from mismatched parentheses, though. Doesn't look like that case has a test, so one should probably be added.
There was a problem hiding this comment.
Updated to be explicit when encountering invalid parens and added test cases
| def _split_css_declarations(style: str) -> list[str]: | ||
| declarations = [] | ||
| current = [] | ||
| quote: str | None = None |
There was a problem hiding this comment.
Nitpick: might be slightly easier to read if quote had exact literal typing.
There was a problem hiding this comment.
Yeah if it was typescript i would, but mypy is not so smart - can be done but you have to be much more explicit. e.g.
if char in {"'", '"'}:
quote = char
will not pass anymore as doesn't refine the type, have to split it to
if char == "'":
...
elif char == '"':
...
This supports both as a prop directly to {% component %}, and
nested within on html tags
Resoles #98
davecoates
force-pushed
the
fix/issue98-style-prop-handling
branch
from
6c6e582 to
e489376
Compare
2 participants
This supports both as a prop directly to {% component %}, and nested within on html tags
Resoles #98