This repository has been archived by the owner. It is now read-only.

Fork with a friendlier stripper #11

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
7 participants
@janlimpens

I created a stripper that produces human friendlier output. Are you interested in this?

@jamiew

This comment has been minimized.

Show comment
Hide comment
@jamiew

jamiew Feb 19, 2013

This should really be in markerb core! Great fork

jamiew commented on 06029a0 Feb 19, 2013

This should really be in markerb core! Great fork

+module Redcarpet
+ module Render
+
+ class RefinedStripper < StripDown

This comment has been minimized.

@lucasmazza

lucasmazza Mar 24, 2013

Contributor

This class should be named Markerb::TextRenderer or something on that line.

@lucasmazza

lucasmazza Mar 24, 2013

Contributor

This class should be named Markerb::TextRenderer or something on that line.

This comment has been minimized.

@lucasmazza

lucasmazza Mar 24, 2013

Contributor

Also, it took me a while to understand what this class does. What do you think about adding some documentation to it explaining that it strips down some markdown tags so the raw text can me more human friendly?

@lucasmazza

lucasmazza Mar 24, 2013

Contributor

Also, it took me a while to understand what this class does. What do you think about adding some documentation to it explaining that it strips down some markdown tags so the raw text can me more human friendly?

+ end
+ end
+
+ def list_item *args

This comment has been minimized.

@lucasmazza

lucasmazza Mar 24, 2013

Contributor

This method definition should have parentheses since it accepts an argument.

@lucasmazza

lucasmazza Mar 24, 2013

Contributor

This method definition should have parentheses since it accepts an argument.

@@ -17,10 +19,11 @@ def call(template)
if template.formats.include?(:html)
"Redcarpet::Markdown.new(Markerb.renderer, Markerb.processing_options).render(begin;#{compiled_source};end).html_safe"
else
- compiled_source
+ "Redcarpet::Markdown.new(Markerb.text_renderer, {}).render(begin;#{compiled_source};end)"

This comment has been minimized.

@lucasmazza

lucasmazza Mar 24, 2013

Contributor

I think you don't need to supply the empty hash here.

@lucasmazza

lucasmazza Mar 24, 2013

Contributor

I think you don't need to supply the empty hash here.

@lucasmazza

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Mar 24, 2013

Contributor

Sorry about taking so long to check this one.

The feature looks promising, but we need to get along some bits before it, so I added some comments on the code. Also, github says it can't be automatically merged, so you need to pull the latest changes from master to it before we can merge the PR.

Contributor

lucasmazza commented Mar 24, 2013

Sorry about taking so long to check this one.

The feature looks promising, but we need to get along some bits before it, so I added some comments on the code. Also, github says it can't be automatically merged, so you need to pull the latest changes from master to it before we can merge the PR.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Mar 25, 2013

Collaborator

I'm not sure about this change. It override some internal of Redcarpet and since Redcarpet can be deprecated it will this will make harder to change it to something else in the future.

See vmg/redcarpet#212

Collaborator

rafaelfranca commented Mar 25, 2013

I'm not sure about this change. It override some internal of Redcarpet and since Redcarpet can be deprecated it will this will make harder to change it to something else in the future.

See vmg/redcarpet#212

@rubytastic

This comment has been minimized.

Show comment
Hide comment
@rubytastic

rubytastic Nov 15, 2013

Having exact this same issue, can this be merged?

Having exact this same issue, can this be merged?

@odigity

This comment has been minimized.

Show comment
Hide comment
@odigity

odigity Apr 11, 2014

Best. Issue Title. Ever.

odigity commented Apr 11, 2014

Best. Issue Title. Ever.

@janlimpens

This comment has been minimized.

Show comment
Hide comment
@janlimpens

janlimpens Apr 11, 2014

Ofer Nave:

Best. Issue Title. Ever.


Reply to this email directly or view it on GitHub.

Thanks, first one to notice ;)

Ofer Nave:

Best. Issue Title. Ever.


Reply to this email directly or view it on GitHub.

Thanks, first one to notice ;)

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva May 6, 2014

Collaborator

Thanks, but I don't think we should have that into markerb. We could, however, have a text renderer option, pretty much as we have the renderer already (which will be in fact html renderer), and allow you to set any renderer you want. This way you could just configure it and have this code in your app, and customize it as you want. I think this is a better path going forward.

I'm giving this a close for now, thanks for your contribution.

Collaborator

carlosantoniodasilva commented May 6, 2014

Thanks, but I don't think we should have that into markerb. We could, however, have a text renderer option, pretty much as we have the renderer already (which will be in fact html renderer), and allow you to set any renderer you want. This way you could just configure it and have this code in your app, and customize it as you want. I think this is a better path going forward.

I'm giving this a close for now, thanks for your contribution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.