Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test fixes #4

Merged
merged 2 commits into from
Oct 14, 2019
Merged

Test fixes #4

merged 2 commits into from
Oct 14, 2019

Conversation

gjtorikian
Copy link
Contributor

Heya 👋

Well, it turns out, a lot of the core Commonmarker C library behavior is a bit strange when it comes to parsing/walking the AST. As you've noticed, Commonmarker will just blatantly rewrite text as it sees fit. Wrote a list using *s? Too bad, Commonmarker prefers - after it's done parsing. I found an ancient PR to fix this specific behavior, but you already found similar defaults in other things like blockquotes and code blocks. So according to the C code, there's literally no way to just "pass through" text as it exists, which really sucks.

Given that, if the text diff needs to remain identical, it might well be that the only plausible solution is to do a gsub as you originally thought.

In this PR I'm suggesting an alternative approach, though it's by no means fool-proof. Here, rather than passing all unmatched text over with node.to_commonmarker (which sometimes rewrites the content), we'll instead keep an array pair of the original text and its replacement, for node types we care about. So, if headers are the only thing that need to change, a header method can operate on the node, and then an iteration over the changes can make those substitutions. It passes all the original test cases, so if header manipulation is all that's changing, it should at least be a viable solution for this!

@hannesfostie hannesfostie merged commit 1aaeb52 into hannesfostie:master Oct 14, 2019
@hannesfostie
Copy link
Owner

This is awesome, I'll merge it to keep the commit history around (your first commit is very interesting too, and explains so much). I've ported this to our app and it looks like we're just about ready to send things to our translation vendor.

Thank you so much for all your help!

@gjtorikian gjtorikian deleted the test-fixes branch October 14, 2019 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants