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

Provide context for renderer #4

Closed
wants to merge 2 commits into from
Closed

Provide context for renderer #4

wants to merge 2 commits into from

Conversation

gholt
Copy link

@gholt gholt commented Mar 2, 2014

Great work on this; it is indeed fast!

Would you consider a change something like the prototype in this pull request? It provides the inline_lexer to the renderer method and sets a couple extra values that can be used for more advanced rendering.

Of course, a full implementation would change all the renderer methods to accept the lexer as an argument and would be breaking backwards compatibility, but it is a very new project so maybe it is in time? If so, I'm willing to make the change and submit it.

In the exact example usage I'm thinking of, I detect if an image is all alone or surrounded by other elements. Depending on its surroundings or lack thereof, I change the rendered CSS class. So for example, an image followed by text all in one paragraph might float the image left and flow the text around it.

Maybe an image would better explain:

sample

@lepture
Copy link
Owner

lepture commented Mar 2, 2014

It's a good idea for providing a context to renderer. But I am not with the solution. How about pass the whole markdown parser to renderer:

# Markdown
self.renderer = Renderer(self, **kwargs)

# Renderer init
def __init__(self, parser, **kwargs):
    self.parser = parser

@gholt
Copy link
Author

gholt commented Mar 2, 2014

I considered that, but the code looks a little strange:

renderer = MyOwnRenderer()
parser = Markdown(renderer=renderer)
renderer.parser = parser
output_text = parser.render(input_text)

However, I can completely work with that; I would just need the code that sets first_item and has_more_items (or whichever names are preferred).

@gholt
Copy link
Author

gholt commented Mar 2, 2014

Also, are you planning to retain marked compatibility as closely as possible? If so, should we ask @chjj for the same change upstream? I don't use node.js myself, but maybe they would find it useful too?

@lepture
Copy link
Owner

lepture commented Mar 3, 2014

@gholt No. mistune's API is more like misaka.

lepture added a commit that referenced this pull request Mar 3, 2014
@lepture
Copy link
Owner

lepture commented Mar 3, 2014

@gholt You don't need first_item, has_more_items. Check hasattr(lexer, '_match') for detecting first_item.

@lepture lepture closed this Mar 3, 2014
@gholt
Copy link
Author

gholt commented Mar 3, 2014

Cool, even better; thanks!

@lepture
Copy link
Owner

lepture commented Mar 3, 2014

@gholt Maybe you can share your idea with the custom renderer. I'll add a link in the documentation.

@gholt
Copy link
Author

gholt commented Mar 3, 2014

Hmm. I just updated and tried out the _match -- only problem is I can't detect when an item is the first item or am I missing something?

@lepture
Copy link
Owner

lepture commented Mar 3, 2014

@gholt Why do you need first_item? How would you use it?

@gholt
Copy link
Author

gholt commented Mar 3, 2014

Being able to detect the difference between:

Text ![img](img) text.
Text text. ![img](img)
![img](img) Text text.

and

![img](img)

Text text.

lepture added a commit that referenced this pull request Mar 4, 2014
@lepture
Copy link
Owner

lepture commented Mar 12, 2014

Use line_match and line_started

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.

None yet

2 participants