Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 1.5.0a1

Bug #10266 List in @todo tags not processed correctly
Submitted: 2007-03-05 15:50 UTC
From: yunosh Assigned: ashnazg
Status: Closed Package: PhpDocumentor (version CVS)
PHP Version: 4.4.5 OS:
Roadmaps: 1.3.2    
Subscription  


 [2007-03-05 15:50 UTC] yunosh (Jan Schneider)
Description: ------------ The following tag: * @todo - Lots of stuff needs to be tested. * - Definitely have to go through DIME spec and make things work * right, most importantly, sec 3.3. * - Make examples, document. creates the following output: # todo: * Definitely have to go through DIME spec and make things work right, most importantly, sec 3.3. * Make examples, document. While this tag: * @todo foo * - Lots of stuff needs to be tested. * - Definitely have to go through DIME spec and make things work * right, most importantly, sec 3.3. * - Make examples, document. renders correctly to: # todo: foo * Lots of stuff needs to be tested. * Definitely have to go through DIME spec and make things work right, most importantly, sec 3.3. * Make examples, document.

Comments

 [2007-03-05 17:45 UTC] ashnazg (Chuck Burgess)
Judging from the behavior, it looks like the proper way to give an item list to the @todo tag is either: * @todo The TODO List * - item 1 * - item 2 or * @todo * The TODO List * - item 1 * - item 2 which both produce # todo: The TODO List # o item 1 # o item 2 This format seems to satisfy the @todo tag's syntax requirement of an "informational string" (http://pear.php.net/package/PhpDocumentor/docs/latest/phpDocumentor/tutorial_tags.todo.pkg.html) while separately satisfying the item list format requirement of beginning each list line with the list character and in the same column position (http://pear.php.net/package/PhpDocumentor/docs/latest/phpDocumentor/tutorial_phpDocumentor.howto.pkg.html#basics.desc). Do we need to add additional examples in the documentation (in both places I list above) to better communicate this? Or should the todo tag be accepting an item list as its "informational string"?
 [2007-03-05 18:18 UTC] yunosh (Jan Schneider)
> Do we need to add additional examples in the documentation > (in both places I list above) to better communicate this? Yes, it's not obvious at all what an "information string" is. > Or should the todo tag be accepting an item list as its > "informational string"? That would be the best solution for me personally.
 [2007-03-05 18:26 UTC] cellog (Greg Beaver)
also possible is to use multiple @todo tags,fyi However, this is a bug, the simple list processor is not recognizing the simple list. I think the reason is that it ltrim()s the string, and simple lists must begin with a space
 [2007-03-14 12:51 UTC] ashnazg (Chuck Burgess)
When testing against this example: * @todo - item 1 * - item 2 the "item 1" part gets lost when the docs are created, BUT they are still visible in the sourcecode view that is generated. I THINK this means it's not being dropped by the parser, but by the converter...
 [2007-03-14 13:19 UTC] ashnazg (Chuck Burgess)
Further, comparing the way the different output converters display the resulting list, it almost looks like the "list" object is being given "- item 1" as its "title", which gets misinterpreted and dropped, but the list still gets initialized, because when it receives "item 2" it knows its a list item. Therefore the list looks like this: # * item 2 where the empty "item 1" started the outer level list (denoted by the # position), and "item 2" started an inner level list (denoted by the * position). Just hypothesis and observation so far... I should probably also back up a little from earlier when I said the entire todo list is visible in the filesource view. It's visible, but not EXACTLY aligned. There is an extra space in the "item 2" line. If the initial parser is reading it that way, and giving it to the HighlightParser and Converter that way, it would explain why the Converter is seeing "item 2" as an inner level list rather than being the same level as "item 1". The hunch that the Converter is later dropping the "item 1" text may be a separate issue completely. Then again, maybe its the initial parser's fault for giving it to the converter that way? If so, then the one problem may indeed be with the parser, and the highlightParser and converter just hiccup on it differently.
 [2007-03-14 13:27 UTC] ashnazg (Chuck Burgess)
Greg/Josh, am I understanding the program flow correctly? I'm thinking the intermediate parser hands off its results to the converter, and hands off the same results to the highlight parser in parallel to the converter: parser -> intermediate parser -> converter ..........................\----> highlight parser Or, do the intermediate and highlight parsers actually run parallel to each other, receiving the same initial input from the first parser? parser ...\---> intermediate parser -> converter ...\---> highlight parser
 [2007-03-14 18:47 UTC] ashnazg (Chuck Burgess)
Tracked down the spot where the todo's text value was first being sent to the simple list handlers. Added code to recognize that a parserTag tag's value text starts with a simple list iterator, and adds back enough left-pad spaces to get the iterator's column position lined back up with where it originally is in the code. That's in the DocBlockTags patch, along with a small refactor in ParserDescCleanup.inc to get the list of valid list iterator characters into one convenient place. This appears to solve the reported problem in the generated docs.
 [2007-03-14 21:06 UTC] ashnazg (Chuck Burgess)
Figured out what was causing the second (and all subsequent) lines of the todo's simple list to be indented one extra space. That fix is in HighlightParser-patch.
 [2007-03-14 21:07 UTC] ashnazg (Chuck Burgess)
Jan, can you apply these patches to your install and ensure they work for you? Josh/Greg, how do the patches look to you?
 [2007-03-14 21:13 UTC] jeichorn (Joshua Eichorn)
I don't see anything wrong with the changes, lets at least get a example file with all the various formats into cvs for testing.
 [2007-03-14 22:09 UTC] yunosh (Jan Schneider)
I get a lot of the following notices, but the result is fine. It could be a bit more beautiful if the list wasn't wrapped in another list, but that's just cosmetics. Notice: Undefined offset: 0 in /usr/share/php/PhpDocumentor/phpDocumentor/DocBlockTags.inc on line 86 Notice: Undefined offset: 0 in /usr/share/php/PhpDocumentor/phpDocumentor/DocBlockTags.inc on line 87 Notice: Undefined offset: 0 in /usr/share/php/PhpDocumentor/phpDocumentor/DocBlockTags.inc on line 88
 [2007-03-15 13:15 UTC] ashnazg (Chuck Burgess)
If by "list wrapped in another list" you mean the way it appears on the overall TODO webpage (rather than the page for the individual class/file it's tagged inside), then you have to understand that that "outer" list item is the @todo tag itself. Since the overall TODO webpage is a "list of TODOs", and you're wanting to put a list inside a single TODO, then the "list of TODOs" that have "lists in each TODO" will then result in "list wrapped in another list". If you're really wanting individual list items to be peer-level with each other on the overall TODO webpage, you'll need to use an @todo with each list item in the docblock instead of using one @todo containing a list. I'll be updating the documentation examples once the patches are finalized, to help demonstrate as many variations as I can without making that doc page too big.
 [2007-03-15 13:31 UTC] yunosh (Jan Schneider)
I didn't realize that of course you can have several @todo tags in on file, and since todos are grouped by file, we need a two-level todo list. That makes sense.
 [2007-03-15 13:40 UTC] ashnazg (Chuck Burgess)
Some additional testing this morning shows me that the patches are not fully handling additional list layout variations that I've added to my testfile. Nesting an inner list in an outer list item is not working... the nesting doesn't happen, and the last item of the inner list disappears. At least, in the generated docs this is the case... the nesting lineup and all list items are fine in the sourcecode view. The problem is I see with the sourcecode view is that on PHP5 versions the leading "*" docblock marker in the list lines is missing, as well as the final "*/" missing. This happens on PHP5 but not PHP4, presumably because in PHP5 more precise object highlighting is done. Running the same testfile against both v1.3.1 and the current CVS HEAD, I see the same problems with nested lists, so my patches did not introduce the problem. However, the issue with sourcecode missing the leading asterisks DOES appear to be caused by my patch (again PHP5 only). So, I believe the issue with nested list handling is a separate bug that is unaffected by my patches. Further, I think my HighlightParser patch needs more work. However, I've not been able to trigger any of those "undefined offset" errors that Jan reported. They're possibly being caused by a different tag... expanding my test codebase would have been my next step had the initial patch testing looked better. I'm uploading the testfile here that I've been using.
 [2007-03-15 13:52 UTC] ashnazg (Chuck Burgess)
Jan, Using individual @todo tags per list item in your docblocks would still result in the overall TODO webpage grouping your items by file. (ignore the tildas (~) below... I have to use them for spacing to line up the columns like I want you to see them... this HTML form strips leading spaces from each line, so spaces alone won't let me show you the column format I want to display). These docblocks in file1 and file2: /** ~* file 1 ~* @todo item 1 ~* @todo item 2 ~*/ /** ~* file 2 ~* @todo item 3 ~* @todo item 4 `*/ would result in the overall TODO webpage looking like: TODO LIST ~ ~ file1 ~ o item 1 ~ o item 2 ~ ~ file2 ~ o item 3 ~ o item 4 Whereas what you're doing now: /** ~* file 1 ~* @todo - item 1 ~* - item 2 ~*/ /** ~* file 2 ~* @todo - item 3 ~* - item 4 `*/ results in: TODO LIST ~ ~ file1 ~ o o item 1 ~ o item 2 ~ ~ file2 ~ o o item 3 ~ o item 4 Would using multiple @todo tags give you the layout you're really looking for?
 [2007-03-15 14:15 UTC] yunosh (Jan Schneider)
Yes. But your missed one example which would explain why this is probably not possible: /** ~* file 1 ~* @todo - item 1 ~* - item 2 ~*/ /** ~* still file 1 ~* @todo - item 3 ~* - item 4 `*/ results in: TODO LIST ~ ~ file1 ~ o o item 1 ~ o item 2 ~ o o item 3 ~ o item 4
 [2007-03-15 14:24 UTC] ashnazg (Chuck Burgess)
Ah, yes, if you're wanting to group TODO lists per docblock per file, rather than just grouping TODO lists per file. If I were needing to specifically do that, I'd do it this way: ** ~* class 1 in file 1 ~* @todo Laundry List for Class 1 ~* - item 1 ~* - item 2 ~*/ /** ~* class 2 in file 1 ~* @todo Laundry List for Class 2 ~* - item 3 ~* - item 4 `*/ which results in: TODO LIST ~ ~ file1 ~ o Laundry List for Class 1 ~ o item 1 ~ o item 2 ~ o Laundry List for Class 2 ~ o item 3 ~ o item 4 ~ ~ file2 ~ (etc) Having some minimal "title" that leads into the simple list seems to make all the difference, to the current un-Chuck'd PhpDoc code anyway.
 [2007-03-15 14:30 UTC] ashnazg (Chuck Burgess)
Plus, I'd want some minimal "title" for each list, to express which object (docblock) in the file that each list belonged to... otherwise I get the various lists sequentially but with no context (title) to explain why there are separate lists.
 [2007-03-15 14:37 UTC] yunosh (Jan Schneider)
Alternatively, phpdoc could mention the context of the @todo tag, e.g. page-level, class, method, etc.
 [2007-03-15 15:02 UTC] ashnazg (Chuck Burgess)
I've updated the TEST-FILE to contain the exact two simple list examples from the PhpDoc manual (phpDocumentor Tutorial, DocBlock Description details, phpDocumentor/tutorial_phpDocumentor.howto.pkg.html#basics.desc). The second docblock example from the manual (fourth one in TEST-FILE) shows that v1.3.1 and current CVS HEAD do not properly handle nested lists. Again, a separate bug, but somewhat relevant to the discussion here.
 [2007-03-15 15:16 UTC] ashnazg (Chuck Burgess)
Greg/Josh, I'm kind of worried about continuing to try beating this patchset into submission. Instead, I'm inclined to forget about these patches here and just update the manual to indicate that a simple list, when used in a tag's context, should first require a "title" and second require the listed items to begin on a newline. That seems to mitigate the fact that being the tag's "text" means the tag gets stripped from the line before the text gets processed (which throws off the column alignment of the list items)... having the "title" fill the spot of "text" avoids the alignment issue. I haven't seen anything so far in the manual that says a list SHOULD work in a tag without those requirements, so adding them shouldn't "remove" functionality. (If there's somewhere in the manual I've overlooked, let me know so I can figure it in with my thinking.) I'd then open two new bugs, one to cover the extra space in the sourcecode view issue, and a second to cover the failure to handle nested lists. Your preferences?
 [2007-03-15 22:42 UTC] cellog (Greg Beaver)
nested lists are waaay too complicated to handle with a userspace lexer, I grappled with this for ages before giving up. The solution, ultimately, is to use the docblock parser PHP_Parser_DocblockParser, as this is able to process nested simple lists, but requires the docblock extension. However, <ol> nested lists should work just fine.
 [2007-03-16 12:25 UTC] ashnazg (Chuck Burgess)
The only reason I'd brought nested lists into the fray was because they were documented in the manual, and I wanted to verify my patches against what's documented to work. I was disappointed when it didn't work with my patches, but surprised when unpatched code didn't work either. Do I need to remove them from the manual until such time as we get nested list handling for simple lists implemented?
 [2007-03-16 13:49 UTC] ashnazg (Chuck Burgess)
I must have made a bad inference with simple lists, thinking that "different column spacing of the list iterator indicates a different list" would/could/should result in list nesting. That seems to _not_ be the case, and I'm hard pressed to find anything that actually states that. So again, that must be a bad inference I picked up. It looks like using list tags instead of simple list iterators solves all the problems with lists, both in docblock text and in tag text. So, how about this plan: - update simple list description to explicitly state that different spacing does NOT create nesting, and reiterate that it only starts a new standalone list; - state that simple lists CANNOT contain nested lists... otherwise, they're not "simple" anymore; - include new examples showing both simple and tagged lists as the text component of a tag... point out that a simple list in a tag will require a text title and the first item on a newline... also point out that tagged lists are a more robust option to use in a tag; - write up the "fix" for this bug to be the documentation changes, and show a modification of the original bug report using both a titled simple list and a tagged list; - open a new bug on the sourcecode highlighting indenting one extra space on subsequent lines of a tag's text (be it a list or not); Does _this_ sound like a plan?
 [2007-03-19 19:17 UTC] ashnazg (Chuck Burgess)
Guys?
 [2007-03-19 21:13 UTC] jeichorn (Joshua Eichorn)
That seems pretty reasonable to me, but you should check with Greg on this one, the list stuff is all him
 [2007-03-29 19:21 UTC] ashnazg (Chuck Burgess)
Since I can't delete my earlier code patches, I've set their latest "versions" to be an empty file stating "WITHDRAWN - do not use this patch file's earlier versions". I'll get to work on the necessary doc patches as outlined in my 3/16 "plan".
 [2007-03-29 20:46 UTC] ashnazg (Chuck Burgess)
Added patch to the howto.pkg file, adding the additional documentation pieces I had listed in my 3/16 plan.
 [2007-03-29 20:54 UTC] ashnazg (Chuck Burgess)
The "fix" for the original example would be to include a list title for the simple list, or else utilize a tagged list: * @todo The Simple TODO List * - Lots of stuff needs to be tested. * - Definitely have to go through DIME spec and make things work * right, most importantly, sec 3.3. * - Make examples, document. or * @todo * <ul><li>Lots of stuff needs to be tested.</li> * <li>Definitely have to go through DIME spec and make things work * right, most importantly, sec 3.3.</li> * <li>Make examples, document.</li></ul> I've updated the documentation explaining lists (in the Tutorial) to more fully explain the constraints and limitations of using simple lists... see the howto-patch file. I'm hoping this will all suffice to solve this bug.
 [2007-04-09 17:54 UTC] ashnazg (Chuck Burgess)
The documentation fix has been committed to CVS Head as well as the v1.3.2 release branch.