👌 IMPROVE: Ensure len(state.src) == len(state.srcCharCode)

[geebos]

def normalize(state: StateCore) -> None:

    # Normalize newlines
    string = NEWLINES_RE.sub("\n", state.src)

    # Replace NULL characters
    string = NULL_RE.sub("\uFFFD", string)

    state.src = string

When updating state.src, state.srcCharCode should be updated synchronously.

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

[codecov]

Codecov Report

Merging #108 (4de72a8) into master (2136ab7) will decrease coverage by 0.02%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
- Coverage   96.49%   96.46%   -0.03%     
==========================================
  Files          69       69              
  Lines        2912     2916       +4     
==========================================
+ Hits         2810     2813       +3     
- Misses        102      103       +1     

Flag	Coverage Δ
pytests	96.46% <87.50%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
markdown_it/rules_core/state_core.py	100.00% <ø> (ø)
markdown_it/rules_inline/state_inline.py	98.87% <ø> (-0.02%)	⬇️
markdown_it/ruler.py	83.92% <83.33%> (-0.04%)	⬇️
markdown_it/rules_block/state_block.py	98.54% <100.00%> (ø)

---

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 2136ab7...1b8905d. Read the comment docs.

[hukkin]

Nice! I made suggestions that use the @property decorator to keep API identical to JS markdown-it.

With those changes I believe we can also remove line 10 from the __init__ that sets self.srcCharCode.

[hukkin]

Also, might make sense to add the @property to StateBase base class only, to get identical behavior in all StateBase subclasses.

[geebos]

Also, might make sense to add the @property to StateBase base class only, to get identical behavior in all StateBase subclasses.

Nice suggestions! I have added the @property to StateBase. In addition, I have added srcCharCode to @property to avoid assigning srcCharCode from outer codes.

[hukkin]

We should be able to remove the line

self.srcCharCode = [ord(c) for c in src] if src is not None else []

in StateInline.__init__.

In StateBlock.__init__ we can make the lines

        self.src = src
        if srcCharCode is not None:
            self.srcCharCode = srcCharCode
        else:
            self.srcCharCode = [ord(c) for c in src] if src is not None else []

something like

        if srcCharCode is not None:
            self._src = src
            self.srcCharCode = srcCharCode
        else:
            self.src = src

This will keep the (presumably) performance improvement in place.

[hukkin]

Looking good @geebos ! Did you see this comment #108 (review) . You think those two changes can be used here too?

[geebos]

Looking good @geebos ! Did you see this comment #108 (review) . You think those two changes can be used here too?

Thanks for reminding. I have added a set_underline_src method in StateBase, because we cant set _src directly in StateBlock.

[hukkin]

we cant set _src directly in StateBlock.

What makes you think so? I believe this should work just fine as long as there's only one preceding underscore (_src), not two (__src). In case it's mypy that's giving you trouble, you can type annotate self._src in StateBase after the class declaration like

class StateBase:
    _src: str

[geebos]

You are right. I confuse them.

[hukkin]

Nice, this looks very good to me!

[chrisjsewell]

thanks!

[welcome]

Congrats on your first merged pull request in this project! 🎉


Thank you for contributing, we are very proud of you! ❤️