search

wooorm / markdown-rs

CommonMark compliant markdown parser in Rust with ASTs and extensions
https://docs.rs/markdown/1.0.0-alpha.21/markdown/
MIT License
965 stars 53 forks source link

Fix padded code (text) in mdast #123

Closed yshavit closed 3 months ago

yshavit commented 3 months ago

Strip one space from the beginning and end of InlineCode if (a) the value starts and ends with a space and (b) is not all spaces.

From CommonMark spec section 6.1:

If the resulting string both begins and ends with a space character, but does not consist entirely of space characters, a single space character is removed from the front and back.

Resolves #122

wooorm commented 3 months ago

whether this should apply to all inline code text, or just `

How do you mean? Isn’t all “inline code” / “code (text)” with `?

whether I should adjust the position fields (and if so, which? all of them, or just the *_offsets?) to account for the stripping the spaces

I don’t think that’s needed. Positional info represents the whole, which includes the backticks too. So the spaces have no effect on it.

apologies if this PR is not the appropriate way to do that!

This is great! Thank you!

yshavit commented 3 months ago

whether this should apply to all inline code text, or just `

How do you mean? Isn’t all “inline code” / “code (text)” with `?

I think this the raw_text also covers $$, but I'm not 100% — I haven't tested it out yet. And I'm also not sure whether $$ also requires stripping the space.

whether I should adjust the position fields (and if so, which? all of them, or just the *_offsets?) to account for the stripping the spaces

I don’t think that’s needed. Positional info represents the whole, which includes the backticks too. So the spaces have no effect on it.

Awesome, thanks! That makes it easy.

apologies if this PR is not the appropriate way to do that!

This is great! Thank you!

Glad to hear it! 😁

Unfortunately, I won't be able to get to it until later tonight (US east coast), but I'm guessing there's no urgency on this from your end. :-)

wooorm commented 3 months ago

$, as in math, has no standard. I intentionally here use the exact same working of ` for $. So :+1: to same workings for both.

No urgency from me! I really appreciate you helping!

yshavit commented 3 months ago

Ah, if they intentionally work the same, then perhaps this PR is ready to go? I'll go ahead and mark it as "ready for review" and subject it to full critique! :-)

wooorm commented 3 months ago

Thanks Yuval! Released!!

yshavit commented 3 months ago

Awesome, thanks so much!