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

Inline structures (literals and roles) #21

Closed
wants to merge 7 commits into from

Conversation

masklinn
Copy link
Contributor

Builds on #19.

masklinn added 7 commits May 14, 2013 10:42
--HG--
rename : rst2rst/tests/__init__.py => rst2rst/tests/test_fixtures.py
this way, removals and additions show the *errors* in the current output
Allows checking multiple failing fixtures, and better reporting on
tests count.
better to fail on them, if no-behavior is desirable should extend SparseNodeVisitor instead
@masklinn
Copy link
Contributor Author

At this point, the test case completely fails:

  • Text wrapping is not applied correctly as inlines are within a paragraph but not a text
  • Spaces aren't inserted between a piece of text and the following inline construct => foo *bar* becomes foo*bar*
  • Odd newlines insertion, might be related to current spacer state

@benoitbryon
Copy link
Owner

I feel the writer uses wrong design to rebuild the document out of the nodes:

  • currently, when it visits or departs a node, it tries to convert it directly to an output string... But sometimes it needs something from the context, or something that is not available yet. We could implement things, but it would require bunches of tricky conditions on spacer and wrapper. Too bad. Poor readability.
  • what if the writer worked on a tree? Visits and departs could re-create nodes, then the tree could be inspected to generate the right ouput.

As examples:

  • Contiguous inline nodes could be concatenated with a space, then wrapped. Instead of each being wrapped and concatenated with tricky spacer conditions.
  • in bullet lists, first we would generate the content (which can include nested lists), then apply wrapper and spacer rules, then add the bullet.

I feel that this design would make the writer much easier to implement... provided we find a readable way to do this.
@masklinn: what do you think about it?

I once tried to implement it for the lists... but failed to get it quickly :(
It could be a separate (and high priority) ticket. This one would depend on it.

@masklinn
Copy link
Contributor Author

I feel the writer uses wrong design to rebuild the document out of the nodes:

Yes I've got similar issues, it feels like trying to hammer round pegs in square holes.

We could implement things, but it would require bunches of tricky conditions on spacer and wrapper. Too bad. Poor readability.

And requires lots of odd special cases, it also makes things much less composable: "parent" nodes need to set up odd contexts in order to get their children to (unwittingly) do the right things, it's very brittle.

Contiguous inline nodes could be concatenated with a space, then wrapped. Instead of each being wrapped and concatenated with tricky spacer conditions.

Yeah I started going that way (paragraph was the only one applying wrapping after collecting its children), but I think I was in too deep (and had broken all vertical spacing between paragraphs and paragraph-containing nodes) so I reverted everything.

Spacers actually ended up being a much bigger problem than line-wrapping: most block-level nodes (all of them but titles IIRC) don't hold text directly but hold paragraphs which do the text-holding (for all inline structures), so if the paragraph can capture its inline content it can easily wrap everything cleanly. But the interaction of spacers between nested block-level nodes didn't work out.

I feel that this design would make the writer much easier to implement... provided we find a readable way to do this. @masklinn: what do you think about it?

Worth a try

@masklinn masklinn closed this Jan 27, 2021
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.

3 participants