Posted by Gary Feldman on September 8, 2006 at 5:39pm
| Project: | API |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | major |
| Assigned: | jhodgdon |
| Status: | closed (fixed) |
Issue Summary
This patch formats the parameter section of the Doxygen documentation produced by API using <dl>. This makes it easier to read, as the parameter name stands out from the descriptions, instead of running into each other. While I was at it, I factored the parameter parsing code out of api_parse_php_file.
My one main question is whether the pattern to extract the parameter name is correct. I used \w, which seems to match the valid PHP name characters including things like è, but I'm not 100% positive about it.
| Attachment | Size |
|---|---|
| parser.inc.patch | 3.07 KB |
Comments
#1
Here's the style file patch (pretty trivial, and for that matter, optional).
#2
coding style needs work--check all the spacing. there's a commented line of code being added. it looks like the preg_match was changed--i'd want neil or jonbob to look that over before committing the new one.
#3
Bumping up to current version of API module.
#4
Just confirmed in the code, this is definitely still a problem.
#5
Let's re-address this. I'm changing the title to be a bit more generic...
Looking at this page as an example:
http://api.drupal.org/api/drupal/includes--common.inc/function/l/7
It seems to me that the parameter names are currently kind of hard to pick out of the Parameters section. They're just formatted in plain text along with the description, currently.
It would be easier to scan if everything that was on the @param line was highlighted in some way -- either as DL/DT/DD as was proposed in the patch above, or simply with a STRONG tag, or something. Or at least if they had a : after them to separate from the text.
#6
I'm going to take this on. Somewhat related to #774850: Formatting for lists..., which makes list formatting have STRONG tag highlighting for the
key: information
format of lists.
#7
Upping priority (marking "major" things that I'd like to see addressed sooner rather than later).
#8
See also
#608124: Link function parameter types and return types to class/interface pages
#9
Here's a solution that adds bold and : to the parameters -- patch and before/after screen shots attached from my test API site (not running the same theme as the api.d.o site, so it may look a little different). I'll need to update some tests...
Note that after applying this patch, you need to touch an API file and run cron to see the formatting change, because the param formatting is done during parsing, not during display. I'm not tackling that in this patch.
Any thoughts?
#10
Here's another screen shot -- this is a function that has class datataype on the parameter and return value. Code docblock:
/*** Function that has classes for parameter and return value.
*
* @param SubSample $parameter
* This parameter should link to the class.
*
* @return SampleInterface
* This return value should link to the interface.
*/
function sample_class_function($parameter) {
}
#11
This looks like an excellent change for usability. The workaround to pass the strong tags through
check_plain()and then replace them for rendering is a bit odd, but it doesn't seem unreasonable, especially since there is lots of sample code that needs to be escaped.#12
I think is would be nice to store parameter data in a table like (did, name, type, documentation) and then render from that on load. This lets us cleanly get every bit ready for display, gives more flexibility in theming, and opens up possibilities for querying that for other interesting info.
#13
I agree, but I think that is beyond the scope of this patch/issue. I'm trying to concentrate on some UI fixes for this round, so let's file some other issues about refactoring the code?
#14
Here's the patch I propose to commit -- the above patch, plus a test added.
Is that OK with you drumm?
#15
Sure, it can be refactored later, especially with tests.
#16
cool! Committed and pushed in that case.
#17
Automatically closed -- issue fixed for 2 weeks with no activity.