Posted by drumm on March 30, 2010 at 6:06pm
| Project: | API |
| Version: | 6.x-1.x-dev |
| Component: | User interface |
| Category: | task |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
We need pages and listings of some sort for
* Class/interface
* Method (share with function?)
* Properties
* Class constants
This code will be copy/pasted from existing documented items, like functions.
(The UI code can be made much better to stop copy/paste coding, but that is another, less critical, issue.)
Comments
#1
I am going to work on this, this week.
#2
I started chipping away at this yesterday, http://drupal.org/cvs?commit=350340. I think there is actually quite a bit to do. One choice I ran into was: should methods be listed on file pages?
#3
drumm: I think probably that files should list classes/interfaces/functions, and classes/interfaces should list methods/member variables.
#4
drumm: Do you want to assign this back to yourself? There are some other issues I can look into instead.
#5
Here are some thoughts on how this could work:
a) If you are using the search box on api.drupal.org, you would see classes, interfaces, methods, and member variables, along with the other things you can see now (functions, files, constants, etc.), coming up in the AJAX auto-complete list.
b) Each class/interface should have its own page, which documents the class as a whole, and lists member functions/variables. Each member function/variable would show its one-liner description and have a link. It would look pretty much like a file page does currently.
c) Each member function/variable page would have a link back to its class/interface page.
d) File pages would list the contained classes/interfaces, but not all the member functions/variables.
Maybe I can make a mockup...
#6
I've noticed a few things with the current version. As a test, I set it to run off includes/database in a D7 code base.
a) On a Topic page, the function list looks like it's out of alphabetical order, because, for instance, Database::startLog comes before DatabaseConnection::arguments, but the class names are not shown on the function list, so it just says
...
startLog
arguments
...
This does not appear to be a problem in the main Functions list, just on topic pages.
b) My feeling is that if we're going to put methods into function lists, then they should show with the class name. E.g. should appear as Database::startLog rather than just startLog.
c) On a method page, the function signature is wacky. Example:
in SelectQuery (in includes/database/select.inc)
public function addMetaData($key, $object)
appears as
public($key, $object)
in the function signature section at the top.
Also the class name is not shown. That might be related to (b).
I'll take a stab at these three issues for now...
#7
RE #6 (c) -- This appears to be due to the token parsing code in api_page_function(), which is assuming the function name is the first thing in the signature.
In my database, the stored signature for that function is
public addMetaData($key, $object)
so the first token is "public", not a function name.
I'm really not understanding why this function goes to all this trouble to re-parse signature -- why not just display what's in the database?
hmmmm...
Oh, I see, it's doing some checking to see if the arguments were present in old versions... sigh.
Well, it will need to be a bit smarter, in that case.
#8
OK, here's a patch -- just use the object name from the api_documentation table instead.
Seems to work fine... See before and after screen shots.
#9
I redid it to loop until the '('.
#10
#11
I think it would be useful to have the class name in there though, and it isn't in the functions table at all. It is in the doc table. That's why I thought it made sense to just use the name from the doc table...
#12
See also:
#718596: Lacking standards for documenting classes, interfaces, methods
where standards for doxygen for classes are being discussed, as well as how they might be rendered in the API module.
#13
OK. I have taken a look at the current state of the API module. Here is a list of things I think we need to do for this issue:
1) I made an API site with the D7 files in includes/database (which has a bunch of classes, interfaces, and bare functions in it). When I go to the functions page, there are multiple functions listed with the same name (see screen shot). I think that each function listed on the functions page should be prefixed by its class/interface -- e.g. WhateverClass::addField (which is the object name in the database) vs. just addField
OR
Maybe we should just list bare functions and not class methods on the Functions page? I'm not sure about this. ???
2) There isn't a Classes and Interfaces page. We need one that would list classes and interfaces.
3) When I click on a method from the functions page, there is no indication what class/interface it's in. See screen shot.
4) As far as I can tell, there are no individual pages for classes/interfaces, which would list the class-level docblock and the included methods/functions. We need them.
5) When you go to a file page or a topic page, the list of functions does not appear to be in alphabetical order. I think they are shown in alphabetical order by the object name, which is something like WhateverClass::addField, but the displayed name is just addField. As in (1), I think we should be displaying the object name. See screen shot.
6) The file/topic pages need to have class/interface sections.
That's all I can think of at the moment...
#14
Clarification...
6) The file pages do list classes/interfaces as a section currently. Good! However if you click on a link, it doesn't take you to a class/interface page (which is item 4). Topic pages do not have class/interfaces sections yet, and should.
#15
7) Hopefully, the class pages would include methods/member variables from inherited classes, if the methods are not overridden. See
http://drupal.org/node/718596#comment-2841688
#16
This is what the CVS version does. It looks horrible and has big holes, but it is a start. So, what should be on a class page, and in what order?
#17
Some samples of how other sites display class doc:
http://java.sun.com/j2se/1.4.2/docs/api/ (click on a class, such as AbstractAction)
http://us2.php.net/manual/en/class.pdo.php
Things I think are essential to put on class page:
- Links to any classes/interfaces that this class extends/implements.
- Code listing.
- List of methods in the class, with links to the doc pages for the methods.
- List of member variables, with links to doc pages for the variables.
[For these last two, the other way to go would be how the Java site does it: put full doc for each method/member variable on the class page, but I personally think it's pretty cluttered and prefer what the PHP site does (list links to the members).]
Things that would be nice to have:
- Class inheritance tree showing where this class comes from (see Java site for example)
- List of classes that extend/implement the class/interface (see Java site for example)
- List of methods inherited from base classes, with links to them (see Java site for example)
- Nav block when you're on a method page that lets you navigate to other methods within this class (see PHP site for example - click on a method within the class to see the nav)
#18
Oh, you also asked about order. I think this would be a good order:
- Signature line -- class Foo extends Bar implements Baz [where foo, bar, baz are links to those class/interface pages]
- (optional) Inheritance tree (like Java site - showing where Bar came from - with each line in inheritance tree being a sig line)
- (optional) List of classes that extend/implement this one (like Java site) - each one being a link to that class's page
- Class description doc (one-liner and longer doc)
- Member variables list
- (optional) List of variables inherited from base classes
- Member method list
- (optional) List of methods inherited from base classes
- Full source code (could be on another tab, as it is for files?)
#19
subscribe
#20
Should lists of classes etc. be presented as blocks (in the way the Java documentation uses frames) for navigation purposes?
#21
I don't really like the framed lists of stuff API navigation. I think think we should aim to do something better, unless the familiarity really does make it the best for most developers.
#22
I'm not in favor of frames either, but I think it would be helpful if you are viewing method foo() to be able to quickly and easily navigate up to the class Bar that contains it.
#23
I was thinking it might make things easier for theming and customization using things like quicktabs, for example. I have no particular preferences, otherwise. Easy enough to add later if required, I suppose.
#24
I'm attaching a png of a mockup for this, per comment 18.
#25
Not shown in the image is inherited methods, and the collapsible block for view source that @drumm has already provided.
#26
Thanks puregin!
#27
I would do the hierarchy with » or something textual. I don't think the whitespace is doing anything useful. (This is not an endorsement of excessively banishing whitespace.)
#28
And I like the lists as
<ul>. In general, I'd like to see less tables in API's browsing, but that's another issue.#29
Hi, I tried to import my project code and found this issue. I tried to expand a little the classes page.
I made some tests, and as this is my first contribution, I guess it need a lot of rework (and I'm open to any suggestions)
From the puregin #24 mockup, it does :
- class hierarchy rendered in html list
- class methods listing in table with description (similar to the function page), but method visibility is missing (protected..)
- added code for constants but they dont seem to be saved (or I miss some doxygen declaration in my test code because I dont use them atm)
I'll see what I can do for other problems when I'll have this patch applied :p (and some help because I dont know where to look everything for)
#30
#31
I made some minor changes for code style and committed #29.
Constants are currently supported as used in tests/sample/classes.php and tested for in tests/api_update_branch_php.test.
Class properties are a big gap in the current code in CVS. I think the hierarchy could be treated better, but it is good enough.
#32
#33
Happy to become a contributer :p Thanks for the commit and the style rework (I noted the 2 drupal coding standard I have hard time with, the closing bracket at the same level as the caller and the space before the append dot)
Got 2 more patches to review :
1. Add block link and callbacks to class list page
2. Code factoring for table theming
3. Class hierarchy rework, splitted theme, added interface listing
4. Fixed constant list on class page
5. Bug fix : in parsing.inc, split is deprecated on php 5.3, changed for preg_split
1. That first part is just declaration for class listing page and link in block, as all the work is already done in api_page_listing.
2. As you added my code with grouping for table rendering, I refactored all those in theme_api_tables
3. Class hierarchy : I saw you completed my class query code with interface, and noticed there was something to do with it. My first code rendered the hierarchy directly in html so it prevented an easy interface listing. And as interfaces can be multiple for a class, I reworked that part. I splitted the hiearchy building and rendering so it's possible to change it easier. I still use the same unordered list, and as the multiple interfaces prevents me to add them to hierarchy, I just list them after the name. Here is the sample display with this code :
- Sample implements SampleInterface
- SubSample implements SampleInterfaceTwo
The unordered list allow to see easily inheritence for the current class. It's also used on doxygen generated files.
4. In database, constants are defined as 'constant' and class constants are defined as 'const'. I'm not sure is this is intended. Added a fix in the patch for class constants list but you'll see if more work is needed here.
5. Small fix that prevents cron parsing with php 5.3.2-1ubuntu4 on new ubuntu 10.4
split -> preg_split
Didn't looked further to see if it could have side effects or can be optimised.
6. Code style question : if (count($row)) or if (count($row) > 0)
7. I also think that classes specific data (methods, variables, constants) shouldn't be on file page. If they would have to be displayed, I guess the best way would be to group them by class, which will end up almost the same as having them one link away. Seeing all is better on small classes, when you have all info directly, but with large classes that would give too many informations to be really usefull I guess.
I tried to add ' AND class_did = 0' to api_page_listing query.
From my tests, it seems ok with constants (as others are declared as const), files, classes, and of course, functions.
I'm not sure for globals and groups, and there was no clear consensus on this one, so it's not in the patch
8. I tried to list remaining problems from this page :
a. List all implemented interfaces (see #17 java doc)
b. Inherited methods, variables and constants, with links to them (see Java site for example)
c. Class Variables
d. Nav block when you're on a method page that lets you navigate to other methods within this class (see PHP site for example - click on a method within the class to see the nav)
e. Declaration text : MyClass extends OtherClass implements CommonInterface
f. Properties
9. I would also like to add :
g. when required, on class method list, add ' overrides ' .l(parentname, parentname::methodname)
h. link to specific version of a method : @see MyClass::MethodName() should be a link to help point specific things in doc (we often need the base function, but that's not always the case)
I will work on the first group (a-d) in the next days, so you can assign that issue to me if you want.
I guess some of these questions could have had their own topics, but as I'm "new" here (as active user) and wasn't sure about the best way to discuss as there is no forum, I just made a list of ideas and questions I had while playing with that module (and they are only ideas atm, that could grow and become independant)
(and sorry for my crappy english, I'm not always sure I am understandable)
#34
#5 parser.inc patch committed with added regexp delimiters. This matches Drupal 7's _filter_autop(), so I assume is fine.
#35
Committed your other changes. Thanks again.
4. I think we are just using what Grammar Parser provides here. So far sharing naming has been doing okay for functions/methods, but I'm not 100% sure about it. I think we probably want to try out sharing naming, 'constant' is preferred.
6. (count($row) > 0). I like to avoid PHP's transparent type casting and be overly-explicit. === is preferred over == too.
7. I went ahead and added AND class_did = 0. Globals won't be in classes, so no problem. We can deal with groups separately when/if it comes up.
8.d. Skip this. We don't have this type of nav block for files, we shouldn't for classes. Maybe add in another issue.
8. c and f are the same, unless I'm missing something.
8.h. Linking should be another issue, which I'll open shortly.
#36
Adding links to inline docs #788786: Add class-related linking to api_link_documentation()
#37
New patch
This patch covers the a-c needs and introduces 4 new functions to help working with data parsing and rendering while preparing my next 2 patches (one for code factoring, and one for block functions listing).
As you added the interfaces display pointing to the class page, I'll open another issue to implement a separated interface page because the patch have unwanted side effects for them.
Done using ctools_static while parsing hierarchy to store interfaces. Just added it to the members list rendering to get the table.
c. Class Properties
I separated class members and inherited members, so they are listed like this : constants, inherited constants, properties, inherited properties, methods, inherited methods.
but I would like to enhance it. see new request (i).
Others
You mentionned block for files data, but I don't think they would be really usefull. But they can be really interresting to give group related and class related functions, as used on php doc. So I opened #793958: Show a class navigation block when on a method/member variable page as I think the API module needs one.
Thanks for opening the issue.
Thanks for commits again, and your additional work
I'll add a patch for those
Nice, some of my project file pages becomes readable now :p
Remaining
As you knows the project better than me, can there be an easy way to get this one, or should we try to recreate it ?
My mistake, I was thinking of visibility. I didn't found that information in database
Could have some similar need in block so I'll wait until I better know what to do there (and for the i. question). (already implemented for block in my test page)
I think the separated lists I implemented in b-c would be the best way to show all of them on the same page, but depending on the class configuration, I'm sure that would be flood somehow on some pages. The idea I'd like to work on if you're OK is to split the list with page tabs :
#38
#39
Another small patch that complete my previous ones :
- code factoring
- #33 patch (count() > 0) fixes
- minor documentation fix
- small bug fix from last patch that prevented summary from getting links
#40
SQP: You can use the Drupal 7 includes/database directory as a sample in the future, if you want to see some realistic classes and interfaces.
One thing I would like to see in the example in #37 is the declaration line from the class, like:
class SubSample extends Sample implements SampleInterface
We do this for functions, and for a developer (this is developer doc after all) it's a faster way to see at a glance what you're looking at than the inheritance tree (which by the way I like).
Other than that, I think this is great!
#41
Committed both. I will be committing smaller changes for a bit. For example, all arrays should be initialized so you never have to check their count before a foreach.
#42
For member lists, I already committed code to merge everything into one list. My thinking is that you want to see everything the class can do, including inherited items. If you can easily see what is inherited from where, that should be good enough of a distinction. No need to have a bunch of tabs and extra navigation.
I feel like api_members() and api_inherited() might be simplified and merged, but I'm too tired to start in on that today.
For code location, I want to standardize this on all pages. Everything but files and classes has code on-page; files is a separate page; classes is currently collapsed on-page. I think collapsed on-page is the way to go. Most people just need documentation, not implementation, so I think collapsed is right.
I want to emphasize that I want to keep navigation simple. More can happen in the next release cycle, once the basics are done. I always want to keep pages focused on what users are looking for, and I don't think that is a lot of lists.
#43
Thanks for commit and rework again :) . I like how you merged the classes lists, but I think inheritance data shouldn't be too close from summary. I suggest to add a css class for it and use float:right (see example images)
e. Declaration text : MyClass extends OtherClass implements CommonInterface
Followed jhodgdon advice and watched how functions declaration work. Functions have a dedicated table where signature is stored. I think we need the same for classes (as code is stored in html in database, I don't wanna parse it)
Maybe use the same table and rename it.
g. When required, on class method list, add ' overrides ' .l(parentname, parentname::methodname)
Can I display the 'overrides ...' (issue g.) text the same way as 'inherited from ...'?
My first and preferred option is to show class name and link to overrided method (as name is the same), but wouldn't it confusing if inherited link to class (as we already got a link to method), and overrides link to the method ?
2nd option is to use the Class::Method but that would be too much I think (already got links to the class on the page, and what we need is parent method)
3rd option is to only show this on method page, but I think it can be important to have it on class page
j. Code tab
Most people just need documentation, not implementation, so I think collapsed is right.
I agree that code text for entire class wouldn't be often used, that's why I wonder if it's the best to always send it to users ?
One click away in both case, easy to find (top of page), and standardized with file if you use the tab.
patch 1 : class inherited members sorting
patch 2 : merge api_render_tables in api_render_members (no longer required since rendering code factoring)
patch 3 : minor doc fix @praam => @param
patch 4 : fixed some minor problems reported by the coder module
coder still complains that those files are missing @file (they have no documentation)
- tests/api_link_code.test
- tests/api_link_documentation.test
- tests/api_update_branch_php.test
jhodgdon : I also use my project code for tests (16 classes as seen on my #29 example). Sample was perfect to show hierarchy with interfaces, but as I'm sure my hierarchy building is ok now, I thank you for the drupal 7 code advice.
#44
That all looks good. Committed. Thanks for the smaller patch files.
Floating those inherited lines to the right looks good.
e. Yes, class declaration should be stored at parse time.
g. Do we actually need overrides on the method list? The method page should certainly have it somewhere.
The method/function page has gotten some attention. Anything else missing there? Constant and property pages probably need some work.
#45
Patch 1 :
#42 : I feel like api_members() and api_inherited() might be simplified and merged, but I'm too tired to start in on that today.
I looked at it, and reworked the inherited. Building members list with a single call (inherited now returns the full sorted list), so it looks cleaner and should be faster.
Inherited is still in a recursive build, so I'm actually stuck with 2 functions, but I guess it should be ok like that.
I fixed the issue I was afraid of (which leaded me to that complicated build to store class object) by using strip tags in patch 2
Patch 2 : issue g.
Added overrides information like option 1 (show class, link function). I'm sure overriding information can be more important than inheritance. (and that was an easy one)
Patch 3 : the css for float right (class declaration is in patch 2)
Patches are really smaller because the big work is done now and I aliased cvs commands so I can break patches more easily.
Sorry about the 'ret' rename, but may I suggest 'themed' :p
See example, class page looks nice. There's just that code part that I think isn't really visible, and breaks the next item (h3) indentation on my new default site with garland and the other with udtheme
#46
Committed those patches. Will probably do a little refactoring on the overrides/inherited links.
#47
- Overrides links go to the overridden method.
- Inherited links go to the parent class.
Seems like both of these should go to the same place.
#48
Seems to me that the best thing would be to have both go to the method?
#49
yes, I even asked what to do before coding... (#42)
I guess both are different :
- inherited : no need link to the parent method, it is the same, we already got it, so I linked the class.
- overrided : overrided would benefit from the direct function link I think.
If you really want to change that, I would suggest :
-Inherited from Class (no link, class is in hierarchy if needed)
-Overrides Class (link to function)
I hope developers won't get hurt that function name is the same, so a link to a function with a class title should work
We also need a link to the overrided method in function page.
#50
I confirmed constant pages work and added property pages. I think everything has a page now.
#51
I did another pass through this and got searching working decently. Calling this done.
#52
Automatically closed -- issue fixed for 2 weeks with no activity.