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

flatten nested inlines following readability criteria, closes #4368 #4554

Merged
merged 1 commit into from
Apr 26, 2018

Conversation

danse
Copy link
Contributor

@danse danse commented Apr 16, 2018

after the refactoring we can apply transformations to inlines in a bottom-up direction, thus it becomes possible to transform inlines flattening them according to the the readability criteria discussed in #4368

test/writer.rst Outdated
@@ -649,8 +649,8 @@ Smart quotes, ellipses, dashes

‘He said, “I want to go.”’ Were you alive in the 70’s?

Here is some quoted ‘``code``’ and a “`quoted
link <http://example.com/?foo=1&bar=2>`__”.
Here is some quoted ‘``code``’ and a `quoted
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there really any reason to do this flattening for Quoted?
The original lines in this test are perfectly valid RST, are they not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, i didn't realise that quotes are not RST inlines. i modified the combine function as you suggested

@danse danse force-pushed the 4368 branch 2 times, most recently from 12ffe78 to 54950c4 Compare April 17, 2018 09:09
nested inlines are not valid RST syntax, so we flatten them following
some readability criteria discussed in jgm#4368.
@danse
Copy link
Contributor Author

danse commented Apr 27, 2018

thanks, work on this has been long. Unfortunately testing this once again on a set of documents i found out that it has a serious issue: the flatten function drops inline parents that are empty.

the related test and the fix are in #4603. i wonder whether it makes sense to add empty images to the test/writer.* document, as it's a case of valid empty parent which can sometimes be overlooked

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