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

BaseRenderer.render_line_break does not work without override #208

Open
victorvess opened this issue Dec 26, 2023 · 1 comment
Open

BaseRenderer.render_line_break does not work without override #208

victorvess opened this issue Dec 26, 2023 · 1 comment

Comments

@victorvess
Copy link

victorvess commented Dec 26, 2023

Ran into an issue implementing a simple renderer for my project - someone submitted from Windows with CRLF data (similar to what is described in Issue #64 ) and caught me out during testing.

I can handle normalizing my own newlines, but I noticed that BaseRenderer.render_line_break just calls render_inner(token) which cannot succeed, because LineBreak token has no children. This feels like a bug - there should be a safer default (space?? empty string??) or something else here.

I'm happy to contribute a PR for this but I'm curious your opinion on the ideal behavior?

@pbodnar
Copy link
Collaborator

pbodnar commented Dec 30, 2023

Hi @victorvess, thanks for the report.

It looks like the problem of missing children on the LineBreak token is there from the very beginnings of the project. But it went unnoticed, because every *Renderer class has got its own implementation of the render_line_break method, implementation which is appropriate for the target document language.

So, I would infer from this, that probably the best ("safest") we can do now is to change BaseRenderer.render_line_break implementation to raise NotImplementedError() (already used elsewhere in the code) or similar, so that it is clear that this method needs to be implemented specifically for a given language.

What do you think?

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

No branches or pull requests

2 participants