markdown-it-py
markdown-it-py copied to clipboard
Implement JS-esque `StateBase.srcCharCodeAt`
Replaces use of StateBase.srcCharCode
with the more JS-esque StateBase.srcCharCodeAt
that can return None
.
I didn't remove StateBase.srcCharCode
for ease of migration because mdit-py-plugins use it. But did deprecate it.
Happy to hear what you think @chrisjsewell . Not sure if this is the solution or migration strategy you want, but it's something :smile:
I haven't measured performance yet. We may want to do that if this is something we want to proceed with otherwise
EDIT: This PR is in response to discussion in https://github.com/executablebooks/markdown-it-py/pull/186
Codecov Report
Merging #190 (6323f8b) into master (d6adf66) will decrease coverage by
0.06%
. The diff coverage is97.89%
.
@@ Coverage Diff @@
## master #190 +/- ##
==========================================
- Coverage 96.17% 96.11% -0.07%
==========================================
Files 61 61
Lines 3267 3267
==========================================
- Hits 3142 3140 -2
- Misses 125 127 +2
Flag | Coverage Δ | |
---|---|---|
pytests | 96.11% <97.89%> (-0.07%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
markdown_it/ruler.py | 88.88% <83.33%> (-0.77%) |
:arrow_down: |
markdown_it/helpers/parse_link_label.py | 100.00% <100.00%> (ø) |
|
markdown_it/rules_block/blockquote.py | 100.00% <100.00%> (ø) |
|
markdown_it/rules_block/fence.py | 100.00% <100.00%> (ø) |
|
markdown_it/rules_block/heading.py | 100.00% <100.00%> (ø) |
|
markdown_it/rules_block/hr.py | 100.00% <100.00%> (ø) |
|
markdown_it/rules_block/html_block.py | 100.00% <100.00%> (ø) |
|
markdown_it/rules_block/lheading.py | 98.21% <100.00%> (+0.03%) |
:arrow_up: |
markdown_it/rules_block/list.py | 98.86% <100.00%> (+0.01%) |
:arrow_up: |
markdown_it/rules_block/reference.py | 97.70% <100.00%> (ø) |
|
... and 15 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update d6adf66...6323f8b. Read the comment docs.
Seeing that this PR only removes 4 instances of try: ... except IndexError:
handling and that almost always we do an if pos < maximum:
check or similar to ensure None
is not an option, I actually still think it may be a fine decision to reject this change, and simply be more strict/explicit about out-of-range indices than upstream is. The current state is also better for performance. Arguably we'd be throwing away some work that has gone into making upstream more type safe.
The downsides for rejecting are diverging slightly from upstream logic, and the chance that there's still some place in the code where None
handling is required but we don't know.