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

Calling super in a subclass of Render::HTML should render the HTML #51

Open
judofyr opened this issue Aug 27, 2011 · 7 comments
Open

Calling super in a subclass of Render::HTML should render the HTML #51

judofyr opened this issue Aug 27, 2011 · 7 comments

Comments

@judofyr
Copy link

judofyr commented Aug 27, 2011

class SCREAMING < Redcarpet::Render::HTML
  def header(text, level)
    super(text.upcase, level)
  end
end

Redcarpet::Markdown.new(SCREAMING).render("# Hello")

Output: NoMethodError: super: no superclass method 'header' for #<SCREAMING:0x007fd0b4035d40>

Expected result: "<h1>HELLO</h1>"

@vmg
Copy link
Owner

vmg commented Aug 28, 2011

This is (unfortunately) the expected behavior. The Render::HTML class is just a thin wrapper for the actual render code written in C. It's not possible to call the C renderer directly, and allowing this would suppose a gazillion lines of boilerplate with very dubious performance results (the rendering pipeline would be like Ruby -> C -> Ruby -> C, which is quite the roundtrip).

If you (or anybody else) is willing to attempt the reverse-wrapping, I'd love to see the final outcome. To me it seems like a very costly problem that gives little benefit. Sorry!

@mrzor
Copy link

mrzor commented Sep 22, 2011

I've been bitten by the same issue recently, and it got me thinking about a possible pure ruby implementation for the base HTML/XHTML renderers. Besides providing a nice venue for RDoc, it would provide a way to get a functional equivalent of super in this case.

@vmg
Copy link
Owner

vmg commented Sep 22, 2011

Yes, a native implementation of the renderer would be pretty straightforward and let you use method overloading. It is however a tad cumbersome to write... If you're up for the task, I'd love to have one.

@mrzor
Copy link

mrzor commented Sep 24, 2011

First attempt

So I thought a clever HTMLProxy renderer could do the trick, instead of a pure ruby HTML renderer. I was wrong.

I have to confess I didn't really understand tanoku's "It's not possible to call the C renderer directly" initially. After looking at the C code, especially, rb_redcarpet__overload in rc_render.c, it's clearer.
What's clearer ? "you can't call the C renderer directly. the C renderer calls you."

Proposal

Now, would allowing ruby to call the renderer HTML implementation directly (code from html.c) "suppose a gazillion lines of boilerplate code" ? Could a class be created by the C extension code that maps all the code in html.c to a ruby Class or Module ?

Such a class would allow for super to work, but it would effectively make moot of the clever callback system that is implemented in redcarpet innerds, and as a Render::Base subclass, it would certainly be slower.

Or would it ? By hacking around rb_redcarpet__overload respond_to usage, it might be possible to substitute the ruby callback wrapping the html.c callback by the html.c callback, removing overhead for any call that was not overriden by ruby code in the first place.

  • (+1) renderer code can't diverge (it's the same code)
  • (+0.5) it might still be faster than a ruby emulation of html.c
  • (- 0.5) extra ruby -> C -> ruby magic
  • (- 0.5) code with bad karma ends up in rd_redcarpet_overload

I'm not confident enough in what I've stated here to get started in an implementation straight off. Is it worth it ? Is it impossible because I've overlooked something ?

EDIT

3 weeks later. I only spent one evening on my proposal, C side. It was getting harder, and messy. @tanoku was right about a "gazillion lines of boilerplate". Beware of this proposal.

@sunaku
Copy link
Contributor

sunaku commented Oct 8, 2012

😓 Had to work around this limitation by approximating html.c myself, poorly, in sunaku/md2man@db814f6.

@cameron-martin
Copy link

I would find this quite useful. Instead of removing html, I'd prefer it to render html-escaped. It'd be nice to be able to call super from the raw_html and block_html, otherwise I have to replicate their implementations in ruby.

@MaxLap
Copy link

MaxLap commented Jan 16, 2021

I personally just hit this, I wanted to add a link to the perma-link of a header, similarly to Github's. Without this, I need to reimplement the wheel, which I likely will not do as well as Redcarpet.

I would like to suggest that maybe, there could be a 2nd version of the methods, such as wrap_header or edit_header, which receive an extra argument that would be what the default renderer would do.

Since i'm doing this for a static website generator, I am actually building a whole other Redcarpet::Markdown.new(Redcarpet::Render::HTML)).render(text) from inside the header call to get an equivalent of super. I need to rebuild the text from the level + text, but that's still simpler than a full recode. The "performance" aspect of all of this is a non-issue for that use case.

Regards

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

6 participants