feat: Add parse option for char-based columns in Sourcepos#779
feat: Add parse option for char-based columns in Sourcepos#779kivikakk merged 3 commits intokivikakk:mainfrom
Conversation
Martin005
changed the title
kivikakk
left a comment
kivikakk
left a comment
There was a problem hiding this comment.
Thank you, this looks great! 🤍
| if lc.column == 0 { | ||
| return; | ||
| } | ||
| if let Some(line) = lines.get(lc.line.wrapping_sub(1)) { |
There was a problem hiding this comment.
wrapping_sub cannot produce a useful result if lc.line is zero here (18446744073709551615); it would make more sense to give an underflow error than an out-of-bounds one.
There was a problem hiding this comment.
Oh, that's right, thanks for noticing. Should I cover that in another PR? 🙂
There was a problem hiding this comment.
I generally consider such non-blocking comments as: you can feel free to fix if you like, whether in its own PR or just whatever one you happen to do next; and maybe I will if I happen to write a PR myself sometime soon and remember :)
kivikakk
force-pushed
the
sourcepos_chars
branch
from
056a8f9 to
583d092
Compare
Your tests are always super exhaustive to my eyes; I really appreciate your efforts! |
By not being exhaustive, I meant that the test cases don't test every single node value / extension. And there isn't a case for a long Markdown document with multiple node values. |
2 participants
This PR introduces a new parsing option to report source position columns as Unicode character counts instead of UTF-8 byte offsets.
I created a lot of tests for different node values / extensions, but the tests are not exhaustive. Please let me know if I should cover more things 🙂
Closes #777