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

Touchups - youtube auto-parse url feature #8

Closed TheNotary closed 11 years ago

TheNotary commented 11 years ago

Hey, here's another refactor of touchups. I created 2 more classes, ack!

TagCollection (properly named) is a new class that acts pretty much like an array where you put TagNodes (BBTree builds up tag nodes in these collections). Each individual TagNode of course has it's own TagCollection where it's children go. The class also handles the to_html code that once resided in lib/ruby-bbcode.rb. The lib/ruby-bbcode.rb file is very small.

HtmlTemplate is a class that converts the html template described in the tags/tags.rb file and converts it into the appropriate output as specified by whatever BBCode got parsed. This class is inside lib/ruby-bbcode/tag_collection.rb right now because I just feel plain silly about how many classes I spun this code into. I think it should be in it's own file... or maybe I should lookup how folders are typically used in ruby gems (it's really rare that I see folders in projects). This project deserves a class diagram I think... I'll jot one up here for now.

Also, I marked some commits with the words "ATTENTION" when I felt they were such big decisions that they deserved their own issue, but I didn't want to start a million issues at once, I just wanted to get a good sweep through without needed to slow down for coordination purposes. Maybe that will help you, idk, I've never been on your end of the project. I've had ppl create issues, but never had hackers implement adjustments.

So let me know what's on your mind.

And here's a class diagram, how the classes relate to each other.

TagSifter
  -BBTree
    -TagInfo
    -TagCollection
      -TagNode
      -HtmlTemplate

So that diagram attempts to express that:

The code creates a single TagSifter object, wherein a BBTree element is instantiated.
The BBTree parses the bbcode text, converting that data into discrete TagInfo elements.
The TagInfo elements are then converted into TagNode elements which reside inside TagCollection arrays either the topmost collection in @bbtree or in the various child TagNodes nested inside the @bbtree.nodes.

After all of the bbcode text has been parsed and the data has been extracted into TagNodes, the BBTree#to_html method is called, which 'transversly' calls TagNode#to_html on the BBTree's TagNode collection. It's within this method that the TagNodes are placed into their HtmlTemplate form and html is generated in a somewhat recursive manor for all child nodes within @bbtree.

Pretty cool huh? Let me know your thoughts.

TheNotary commented 11 years ago

Also, if you search through the project directory for "FIXME" and "TODO" you'll see stuff that needs to be addressed by you if you get a chance.

Plus, I implemented that feature where you can now input something like [youtube]www.youtube.com/?v=blah[/youtube] and the user no longer needs to manually parse out the v=blah section.

veger commented 11 years ago

Thanks for your work! Your changes looked good to me, they nicely cleaned the project even further! So I merged them.

Furthermore, I added/fixed some comments. Mostly on methods that did not have a description (and got marked by you that I should have done that ;) ) See https://github.com/veger/ruby-bbcode/commit/ea8ba5840c2f590be09fa02512e6a774b0ef3292

An additional note, the tag_list method, is used to be able to directly modify the default tag list, instead of using the to_html parameters. This is convenient when to_html is used multiple times and it is undesired to add the custom tags over and over again.

The next commit, removes some Rails references, as the project does not require Rails anymore we should just remove it (instead of commenting)

TheNotary commented 11 years ago

Great, I'm extremely excited about his. I've begun working on another project (java, yuck) and so I'm a little preoccupied. I'm going to send a pull request that removes the test/dummy/ folder from the project. That won't mess with anything will it? I never knew what that was for. Reject it if I'm in error and I'll reset things on my end.