veger / ruby-bbcode

Convert BBCode to HTML and check whether the BBCode is valid
http://rubygems.org/gems/ruby-bbcode
MIT License
28 stars 29 forks source link

Have [media]url[/media] decode to a youtube vid? #3

Closed TheNotary closed 10 years ago

TheNotary commented 11 years ago

Hey, I'm noticing the string

"[media]https://www.youtube.com/watch?v=EUcPSerw56M[/media]".bbcode_to_html

Doesn't decode to anything interesting such as maybe:

"<iframe width="560" height="315" src="http://www.youtube.com/embed/EUcPSerw56M" frameborder="0" allowfullscreen></iframe>"
TheNotary commented 11 years ago

I just saw a reference to 'youtube' in the tags file, but I'm getting an error when trying to use it.

'[youtube]E4Fbk52Mk1w[/youtube]'.bbcode_to_html
NoMethodError: undefined method `blank?' for "E4Fbk52Mk1w":String
from /home/name/.rvm/gems/ruby-1.9.3-p374/gems/ruby-bbcode-0.0.3/lib/ruby-bbcode.rb:126:in `block in parse'
veger commented 11 years ago

blank? is defined by Ruby n Rails... Are you using ruby-bbcode without Rails? I guess blank? can be replaced by Ruby specific code to make it more broadly usable

TheNotary commented 11 years ago

Ah, that makes sense, I couldn't figure out why the tests were passing but it wouldn't fly in IRB. I'd encourage including the #blank? method in your gem just in case someone comes along wanting to use the gem in a sinatra app or just wants to do some testing in irb like I was doing. Seem like there's lots of room in lib/ruby-bbcode for more .rb files.

veger commented 11 years ago

Is this still an issue? Or does it get solved with your refactoring (issue #5)? I think I have some spare time Sunday to fix this issue, but if it is not necessary anymore, I'll leave it as it is.

(On a second thought: Maybe it is wise to leave it until after the refactoring is pulled, to prevent problems/conflicts while merging)

TheNotary commented 11 years ago

Yeah don't worry about this issue, I'll handle it after the big refactor. There's also a workaround I noticed for irb to just do require 'rails' and that allows for playing in IRB in the meantime for me.

TheNotary commented 11 years ago

This issue is ready to be closed, although, would you accept the addition of a [media] tag in the @@tags folder. I'm not exactly sure how popular it is, but dreamincode.net (a troll nest) uses it for videos. It might be nice to have a tag that can embed both youtube and vimeo vids.

veger commented 11 years ago

Initially I wanted to just support the 'original' BBCode tags, to prevent everlasting growing of them, resulting in messy support. But in this case, it is really convenient to support both video types in one tag...

Do you already have an idea how much impact this has on the code? (I suppose not too much?)

TheNotary commented 11 years ago

I've kind of been thinking this over in the back of my mind as I work on other things. Here is the optimal way to do it (i think):

Method 1) Indicate special handling and link to method for determining html output in app:
Have an entry in @@tags that looks like:

  :media => {
    :is_omni_tag => true,
    :output_method => "media_tag_formating",
    :description => 'Video',
    :example => '[media]https://www.youtube.com/watch?v=knpD0puolNs[/media]',
    :only_allow => [],
    :require_between => true},

Then, in the app, we would define the method:

class CustomOutputFormatter
  def self.media_tag_formatter(between)
    return ""
  end
end

I think the media tag is a special case. I don't see any other tags coming into existence that would be like this... Unless perhaps someone invents a flash app server where you can embed flash apps/games in another site easily... can't think of anything else.

To implement it, we'd need to read the definition for if :multi_output == true

https://github.com/veger/ruby-bbcode/blob/master/lib/ruby-bbcode/tag_collection.rb#L48-L53

And if it is true, then we'd scan the between text (to determine if it's youtube or vimeo) and write the @opening_html to be what ever is appropriate for that video service.

Here's the main question. Would it be better to define all :html_open definitions inside the tags.rb folder and source those definitions when we do the above procedure, or should we define the :html_open inside the code? I can't totally decide.

Here's a crude pros/cons table (there's prolly a better way to write this in github but w/e)

:: Define the html_open in tags.rb and link to those: : pros -This way keeps all the tag definitions in one place

: cons -This way means we'd have to write [vimeo] tags and [engagemedia], etc., etc. (which could all be put into media.rb to clean up tags's look.)

You know, the more I think about it, that con isn't so much a con as it is a major pro. It allows others to define media tags... they could add a field.

:belongs_to_omni_tag => 'media'

And that would tell our omni_tag parent (the media tag) that it should be consulted as a possible definition. Then programmers/ implementers who need to use obscure tags in their forums can pretty easily write up their own tag definition and it will just work. Plus maybe they'll send us a pull request with proper tests and it can be accepted and we won't even have to do all the typing for it!

Btw, I'll be on other projects for the foreseeable future (the next week, lol) so it's all yours if you have time to do the implementing.

P.S. Here's a list of free upload sites: http://en.wikipedia.org/wiki/Comparison_of_video_hosting_services

But there's also video publishing sites that publish their own content and let you embed them which would work well with this gem.

TheNotary commented 11 years ago

Hey, I did some work after a long while. I can see now why you wanted to keep this project limited to original BB code tags. Things become quite a puzzle as features are added. I just finished working on what I call "multi-tags", where now you can use the [media] tag to present either youtube urls or vimeo urls.

The system is now able to match not only youtube.com links, but also youtu.be links and additional variations. This information is assigned in the tags.rb file and is what gives the power behind the multi-tag. It's pretty exciting when I step back and think about these ideas, ha, but I'm a little bummed this wasn't simpler to implement. It was really confusing reading over the '[media]' tag and noting that it is :multi_tag == true, but then reading over it's between_text and checking to see that it is matched against the supported tags listed. And if it can't find a match to the supplied url, it (very hackishly) goes into the @bbtree and sets the media tag to be treated as :text rather than an :opening_tag.

I wanna sit on the ideas that I put together for a little bit, but I thought I'd show you where I was at with things.

https://github.com/TheNotary/ruby-bbcode/tree/multi-tags

TheNotary commented 11 years ago

Hey, so I pretty much finished the feature, in doing so, I added a couple new tags, flickr, vimeo, veoh... some of those services are really sucky and poorly maintained, so if you want to skim some of them that might be good, but I figured it's best to have at least one competitor to promote internet health.

Also, I've been a bit concerned about the project. I've noticed you haven't made any commits lately. Has my refactoring made it more difficult for you to maintain this code base? If so then I absolutely understand if you'd like to revert things back to the way they were before. I find the changes more helpful to me, but it's not a problem at all if I have to work on this gem from my own repo. The last thing I want is to interfere with your workflow.

veger commented 11 years ago

Hi,

First of all, me not contributing does not have (much) to do with your contributions! I am very busy with my PhD thesis and do not have (much) time to spare for contributing to this gem. (and my personal project that actually intend to use this gem...) So please do not worry about this! Your work is much appreciated :)

The main reason to keep the allowed tag limited, was to provide a basic engine that can be extended with to the task of other that want to use it. If every possible tag gets added, the project becomes bloated and full tag tags that are barely used. (and people (and me) would need to remove them again). But keeping it limited also keeps it more clear, I guess (as you noticed) :)

About the choices of video services: Adding the most popular ones (which are also long-term 'stable') seems the best choice. Personally, I am not really interested in video services (and their corresponding tags). So if you want to make a choice services you think is good to keep, I will back you up. Have a choice is indeed a good thing for the 'internet health'! (and to act as an example how the media tag can be extended) So I would propose to keep at least 2 services.

Cheers!

TheNotary commented 11 years ago

That's great to hear, I was worried I brought harm to the project. I've never gone OO with a project that had such complexity (there are lots of moving parts in this engine) and after coming back to it, it took me a while to "get it" again.

About the media vids, two sounds like a good idea to me, I'll trim the list, going with youtube and... ughh... woah, I just found a new one (suggested by life hacker who mention the term Creative Commons) http://www.dailymotion.com/video/xx5mxo_good-cop-great-cop-pilot_fun.

Otherwise we can go with Vimeo which is more popular, but I have trouble with their search results feel a little lack luster to me. I can't choose, ha, as long as we have youtube.com working and anything else, we're pretty much good to go right? I think I'd lean towards dailymotion, although their service is seeming a little flaky right now. I'll post a new pull request within the next week or 2 at most I hope.