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

Pass unsupported tags instead of dropping them #24

Closed
benbalter opened this issue Mar 5, 2014 · 20 comments
Closed

Pass unsupported tags instead of dropping them #24

benbalter opened this issue Mar 5, 2014 · 20 comments
Milestone

Comments

@benbalter
Copy link

What's the logic behind dropping all unsupported tags, rather than passing them through?

HTML is valid within markdown. If I have a non-markdownable entity (e.g. a script tag) reverse_markdown should pass it through to the output (or at least give me the option to do so).

@xijo
Copy link
Owner

xijo commented Mar 5, 2014

Hey Ben,

I agree, it would be nice to have an option for that. I'll dig into that and let you know, alright?

@benbalter
Copy link
Author

If you could point me in the right direction, would be glad to help. I get stuck on getting a tag back with its attributes as a string. Two cases to handle:

  1. Unknown tag with text inside <span class="foo">bar</span> which we'd want to pass through as is
  2. Unknown tag with known tag inside <span class="foo"><b>bar</b></span> which we'd want to be <span class="foo">**bar**</span>.

@benbalter
Copy link
Author

Coded around the problem by using Nokogiri to modify the HTML before passing to reverse_markdown. Closing since it's no longer an issue for me, but can reopen if you'd prefer.

xijo added a commit that referenced this issue Mar 30, 2014
@xijo xijo added this to the 0.5.0 milestone Mar 31, 2014
@xijo
Copy link
Owner

xijo commented Mar 31, 2014

Will be part of the upcoming next release, thanks again for reporting! 👍

@mbrgm
Copy link

mbrgm commented Apr 3, 2014

@benbalter: How did you achieve passing through an unknown tag? I am facing the exact problem right now.

@benbalter
Copy link
Author

Coded around the problem by using Nokogiri to modify the HTML before passing to reverse_markdown.

I made them known tags.

@xijo
Copy link
Owner

xijo commented Apr 3, 2014

@mbrgm I release 0.5.0 where ignore_unknown_tags = true is the default. Just upgrade to the new version should be enough though.

@benbalter
Copy link
Author

It seems the new default behavior in 0.5.0 is to pass the tags through as HTML (rather than dropping them as it did before). Turning off ignore_unknown_tags raises the error, preventing conversion. If I wanted it to ignore (rather than pass through) unknown tags, would a begin/rescue block be the best route?

@xijo
Copy link
Owner

xijo commented Apr 3, 2014

Sure, catching ReverseMarkdown::UnknownTagError is valid option, although it might be tough to do it at the right spot.
Maybe a third option would be nice to do that, something like

config.unknown_tags = :raise  # Raises the error
config.unknown_tags = :drop   # Drops the these tags
config.unknown_tags = :dump   # Dumps tags as html 

What do you think?

@benbalter
Copy link
Author

I like that approach a lot. I might call it pass or pass_through or similar instead of dump, but 👍.

@xijo
Copy link
Owner

xijo commented Apr 3, 2014

Alrighty! I'll open that issue again and let you know when it's done - shouldn't take too long.

@xijo xijo reopened this Apr 3, 2014
@xijo
Copy link
Owner

xijo commented Apr 3, 2014

There are 4 options now:

config.unknown_tags = :raise        # Raises the error
config.unknown_tags = :drop         # Drops the these tags
config.unknown_tags = :pass_through # Include the html
config.unknown_tags = :bypass       # Ignore tags but handle their children

@mbrgm
Copy link

mbrgm commented Apr 3, 2014

I made them known tags.

Ah, ok.

@mbrgm I release 0.5.0 where ignore_unknown_tags = true is the default. Just upgrade to the new version should be enough though.

Unfortunately I need a ruby gem which does not yet support ~> 2.0.0, so I'll have to stick with 0.4.7 for some time.

Maybe a third option would be nice to do that, something like

config.unknown_tags = :raise  # Raises the error
config.unknown_tags = :drop   # Drops the these tags
config.unknown_tags = :dump   # Dumps tags as html ```

What do you think?

+1 on that, and +1 on :pass_through.

Edit: Few seconds too late ;-). I really like the changes. Great work!

@xijo
Copy link
Owner

xijo commented Apr 3, 2014

@mbrgm I just added 1.9.3 support back in, hope this helps you.

Once travis ran through I'll release 0.5.1 with those changes. Thanks to both of you for participating, it's always more fun if one gets some feedback! 👍

@xijo xijo closed this as completed Apr 3, 2014
@benbalter
Copy link
Author

❤️ that approach. 🚲-shedding, but perhaps skip instead of bypass?

@mbrgm
Copy link

mbrgm commented Apr 3, 2014

Great @xijo. Thanks for adding 1.9.3 support! This gem saves me a ton of work converting HTML documentation to an easiier maintainable markdown version!

@xijo
Copy link
Owner

xijo commented Apr 3, 2014

There are only two hard things in Computer Science: cache invalidation and naming things.

Isn't it? :)

I chose bypass to ensure one understands that the content gets converted. skip could also mean that the whole tag is skipped, so no content conversion takes place, don't you think?

@benbalter
Copy link
Author

👍

@mbrgm
Copy link

mbrgm commented Apr 4, 2014

I just came to the point where I'd want to keep the unknown tag but process its children, i.e. a mixture of :pass_through and :bypass. Do you think it would be feasible to add another option for this case?

@xijo
Copy link
Owner

xijo commented Apr 4, 2014

Hmm, that would be the use for a custom converter I think. You can write and register them for your special needs, I wrote an wiki page about it: https://github.com/xijo/reverse_markdown/wiki/Write-your-own-converter

Please let me know if it worked out for you or if you had trouble with this approach.

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

3 participants