🔧 MAINTAIN: Don't allow None value for StateBase.src

[hukkin]

Javascript implementation doesn't handle null values for StateBase.src, so I think we shouldn't either.

Also, it seems the None handling isn't fully implemented in all StateBase subclasses. E.g. StateInline.__init__ will throw an exception when src is None (from the line self.posMax = len(self.src)) even though there is None handling in the __init__ function before that.

[chrisjsewell]

when you say the javascript doesn't handle it, you do know that javascript handles indexing very differently to python?
As I explained here #50 (comment), does not raise IndexErrors when indexing None or and index that is out of bounds.

You should also look at #32, where this ordinal caching was introduced, and where we specifically introduced the catch of None to fix a bug: #32 (comment)

[hukkin]

you do know that javascript handles indexing very differently to python?

Yeah, I'm no JS expert 😄, but hopefully do know some basics.

does not raise IndexErrors when indexing None or and index that is out of bounds.

Out of bounds will not raise for sure, but I'm pretty sure indexing null does, no?

For instance this line https://github.com/markdown-it/markdown-it/blob/1b204ef54063aa7fde055f0e801e234be0a74d1e/lib/rules_inline/state_inline.js#L20 in the JS implementation will surely raise if src is null (accessing length atribute of null).

What would be the use case for initializing a State with src = None, and if there is one, could that perhaps be replaced by src = ""?

If None support needs to be kept then I think the type annotations should probably be changed to reflect that, and at least this line needs to be able to handle a None.

[hukkin]

I found the MyST-NB usage of StateCore(None, .... All tests seem to pass also when instantiating with an empty string: executablebooks/MyST-NB#277 so it seems at least that particular use doesn't have a hard requirement for src = None.

[codecov]

Codecov Report

Merging #73 (bce699c) into master (c55a243) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #73   +/-   ##
=======================================
  Coverage   96.10%   96.10%           
=======================================
  Files          60       60           
  Lines        3232     3234    +2     
=======================================
+ Hits         3106     3108    +2     
  Misses        126      126           

Flag	Coverage Δ
pytests	96.10% <100.00%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
markdown_it/ruler.py	89.65% <100.00%> (+0.18%)	⬆️

---

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 c55a243...bce699c. Read the comment docs.

[hukkin]

I think this should be a pretty safe merge now that myst-nb has seen a few releases after the usage of StateBase.src = None was removed there, and jupyter book release also uses a myst-nb version that doesn't set the None. What do you think @chrisjsewell ?