PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Sat Aug 08, 2020 4:15 am

All times are UTC - 5 hours




Post new topic Reply to topic  [ 77 posts ]  Go to page Previous  1, 2, 3, 4, 5, 6  Next
Author Message
PostPosted: Thu Dec 02, 2010 7:05 pm 
Offline
DevNet Master
User avatar

Joined: Thu Mar 15, 2007 6:28 pm
Posts: 2765
Location: Redding, California
Project Update

Hey everyone,

Scott (s.dot) been down for a couple of weeks with some sort of "viral syndrome", so send him a nice PM. :)

He passed CryoBB over to me (at least temporarily). I've been intermittently making changes as I get time, and I'm really happy with the results. Here's what I've gotten done.

CryoBB_Tag

CryoBB_Tag has been broken into two classes: CryoBB_Tag, and CryoBB_Tag_Parameter. The new system allows developers to create different types of tags, as long as the ultimately extend CryoBB_Tag. This also moved some code out of Base and into Parameter.

CryoBB_Processor

Developer-defined content processing is different. Instead of passing a callback function, you pass your own object that extends CryoBB_Processor. This is a more OO approach, and allows multiple processing methods. Yes, I know you're thinking "why didn't he make it an interface?". Well, that would mean the dev has to create all required methods, even ones he/she doesn't want.

Code Readability

The regular expression match array has been turned into a stdClass. This makes the code in CryoBB_Base much more understandable (e.x. instead of $match[1], $match->tag).

Parsing Method Change

The way it parses the input has been transformed. Before, it would just replace the match with str_replace. To account for tags who's contents should not be parsed, they were removed and replaced with temporary markers, then restored later. This required a great deal of code. Now, it records the match's offset and length (PREG_OFFSET_CAPTURE), and makes the replacements with substr_replace. This change alone cut down 119 lines of code. Still in the "unstable" branch, as it has not been thoroughly tested.


Anyone interested in this project, I encourage you to check out the changes.

-Jonah


Top
 Profile  
 
PostPosted: Thu Dec 02, 2010 8:07 pm 
Offline
Site Administrator
User avatar

Joined: Wed Aug 25, 2004 7:54 pm
Posts: 13592
Location: New York, NY, US
Great work Jonah!

_________________
(#10850)


Top
 Profile  
 
PostPosted: Tue Dec 07, 2010 10:01 pm 
Offline
DevNet Master
User avatar

Joined: Thu Mar 15, 2007 6:28 pm
Posts: 2765
Location: Redding, California


Top
 Profile  
 
PostPosted: Tue Dec 07, 2010 11:06 pm 
Offline
DevNet Master
User avatar

Joined: Thu Mar 15, 2007 6:28 pm
Posts: 2765
Location: Redding, California
Just created a set of unit test cases. This should make development easier.

P.S. Is everything cool now Josh? Can I keep programming? :)


Top
 Profile  
 
PostPosted: Tue Dec 07, 2010 11:37 pm 
Offline
Tranquility In Moderation
User avatar

Joined: Sun Feb 06, 2005 8:18 pm
Posts: 5001
Location: Indiana
Jonah, things are looking amazing. I know we've collaborated a little bit on the changes but the work credit and generally most credit should definitely go to you!

I was thinking.. this will severely slow down the speed of the library, but, perhaps greatly improve stability and usage. What if we iterate through all of the defined tags and check explicitly for that tag using a regexp?

Gathering all of the tags in one regex might be allowing for too much room for error to occur, and doing it separately would account for the invalid nesting (though it wouldn't fix invalid nesting, it would at least parse all of the tags).

Thoughts?

_________________
- A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.


Top
 Profile  
 
PostPosted: Tue Dec 07, 2010 11:55 pm 
Offline
DevNet Master
User avatar

Joined: Thu Mar 15, 2007 6:28 pm
Posts: 2765
Location: Redding, California
It's my opinion that it's great just the way it is. The output is *always valid XHTML, and the number of situations in which one would need overlapping tags is absurdly miniscule.

* unless the person writing text puts inline tags outside of block tags

Edit: just realized that the tag needs to be cleaned with preg_quote().


Top
 Profile  
 
PostPosted: Wed Dec 08, 2010 12:05 am 
Offline
Tranquility In Moderation
User avatar

Joined: Sun Feb 06, 2005 8:18 pm
Posts: 5001
Location: Indiana
The tag doesn't need to be cleaned because it is being checked with ctype_alnum, and regexp delimiters must be non-alphanumeric characters.

I was thinking the reason for individual regexps for each tag would be to allow for something like:

[ quote="[ url ]http://www.php.net[ /url ]"]something[ /quote ]

I think I might be in shape to start working on this again (taking your direction, since you've took it in an exciting new direction!). I absolutely love how you put the match data and operations in Match.php.. brilliant.

_________________
- A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.


Top
 Profile  
 
PostPosted: Wed Dec 08, 2010 11:34 am 
Offline
DevNet Master
User avatar

Joined: Thu Mar 15, 2007 6:28 pm
Posts: 2765
Location: Redding, California
Oh, yeah. It totally blows up if you do that...

Input
Syntax: [ Download ] [ Hide ]
[quote=[url=http://google.com]bobby[/url ]]hello[/quote]

Output
Syntax: [ Download ] [ Hide ]
<div><strong>[url=http://google.com wrote:</strong></div><blockquote>bobby[/url]]hello</blockquote>


Edit:

Realized why; it's because of the regex.

[quote=[u]Jonah[/u]]Hello, world![/quote ]

The red square brace is detected as the end of the quote tag. So, the quote is by: [u

And [u said: Jonah[/u]]Hello, world!


Top
 Profile  
 
PostPosted: Sat Dec 18, 2010 1:07 am 
Offline
DevNet Master

Joined: Wed Feb 11, 2004 4:23 pm
Posts: 4872
Location: Palm beach, Florida
No matter what I'll be able to break it. Stop trying to code for other programmers and code for the users ;-) the users aren't going to legitimately screw it up that bad, and if they do... by that time all original intent will be lost.

If you need it to be that foolproof to foolish users, I'd suggest a WYSIWYG editor and running the HTML thru htmlpurifier. To try and correct erroneous bbcode would be an exercise of advanced artificial intelligence. The best thing to probably do would be simply detect it, and notify the user.


Top
 Profile  
 
PostPosted: Sat Dec 18, 2010 12:36 pm 
Offline
DevNet Master
User avatar

Joined: Thu Mar 15, 2007 6:28 pm
Posts: 2765
Location: Redding, California


Top
 Profile  
 
PostPosted: Sat Dec 18, 2010 2:45 pm 
Offline
DevNet Master
User avatar

Joined: Thu Mar 15, 2007 6:28 pm
Posts: 2765
Location: Redding, California
I've been working on putting the recursion into the parameter, and it's almost done. You can see what I've done so far at the thread: viewtopic.php?p=637936


Top
 Profile  
 
PostPosted: Sun Dec 19, 2010 4:39 pm 
Offline
DevNet Master
User avatar

Joined: Thu Mar 15, 2007 6:28 pm
Posts: 2765
Location: Redding, California
Okay, the regular expression now fully supports BBCode inside of the parameter. Exciting! Everything works perfectly now. Not only that, but the expression is partially commented now, too. I also went through the code again and reorganized some of the logic, added more comments, and removed a bit of code smell (correct term?). All unit tests pass.

It's all in the trunk. Be sure to update your working copy! :)


Top
 Profile  
 
PostPosted: Mon Dec 20, 2010 2:12 am 
Offline
DevNet Master

Joined: Wed Feb 11, 2004 4:23 pm
Posts: 4872
Location: Palm beach, Florida
The comments above the test methods are more aptly suited for an assertion message on the assertion itself, so its reported in the results if it fails.

Also the context you write these should begin with "should" IMO, in order to document behavior. For example, your comment. "// test tag with parameter". This should be an assertion message, not a comment, and should read "should convert bbcode attribute to an HTML one" (something along those lines).


'// assert with parameter' & '// assert without parameter' you verify these both in one test, which is easy to see why they have similar setup logic. In this case a "test utility method" is more desirable than the "god test". The "god test" achieves a good result w/o duplicating logic. However a "test utility method" is more elegant IMO

Example psuedo-test

Syntax: [ Download ] [ Hide ]
// TEST METHODS (one assertion per method highly desired)
function testShouldDoMath()
{
  $a = $this->setupA();
  $b = 5;
  $this->assertEquals( 3, foo(5,$a), 'should do math');
}

function testShouldGrammar()
{
  $a = $this->setupA();
  $b = 'bar';
  $this->assertEquals( '6bar', foo2(5,$a), 'should do grammar');
}


//  TEST UTILITY METHOD
function setupA()
{
 ... really. long. redundant. logic...
}
}
 


Feel free to add me as a committer and I'll try and clean up the tests.

In the event your setup is more complicated, you could pass the objects back in an array and use list(), use multiple test utility methods, or better yet sprout a new test class. Each test class would contain setup logic in the setup method, never in the test methods themselves. test methods themselves should be very minimal code, only what is necessary to describe an interaction, disregarding "behind the scenes" setup.

Just remember this mantra:

if its important to understanding the test, its important that it is in the test.
If its NOT important to understanding the test, its important that it is NOT in the test.


Sloppy code, production or test code, causes more than just not pretty code. Further reading http://xunitpatterns.com/Test%20Smells.html

I give these unit tests an overall A- my only critiques are mostly minor, and its better than what I see on average.


Top
 Profile  
 
PostPosted: Mon Dec 20, 2010 1:24 pm 
Offline
DevNet Master
User avatar

Joined: Thu Mar 15, 2007 6:28 pm
Posts: 2765
Location: Redding, California


Top
 Profile  
 
PostPosted: Mon Dec 20, 2010 1:57 pm 
Offline
Tranquility In Moderation
User avatar

Joined: Sun Feb 06, 2005 8:18 pm
Posts: 5001
Location: Indiana
Truly amazing work Jonah.
I just set up a custom tag that parses [rainbowtext][/rainbowtext] LOL, it uses the processor to apply a rainbow swatch of colors sequentially to each letter.
Once this thing is stable it'll be out of this world.

_________________
- A google chrome extension. When you search only results from the past year (or set time period) are displayed. Helps tremendously when using new technologies to avoid outdated results.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 77 posts ]  Go to page Previous  1, 2, 3, 4, 5, 6  Next

All times are UTC - 5 hours


Who is online

Users browsing this forum: No registered users and 6 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
cron
Powered by phpBB® Forum Software © phpBB Group