Closed (fixed)
Project:
Drupal core
Version:
7.x-dev
Component:
documentation
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
23 Jul 2013 at 14:06 UTC
Updated:
22 May 2014 at 00:10 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
jhodgdonGood catch -- they probably have the wrong @ingroup or are part of a @defgroup { } or @addtogroup { } that they shoudn't be.
That would probably be a good Novice project to look at the various field-related topics and move the functions around to the right groups. Start with
https://api.drupal.org/api/drupal/core!modules!field!field.module/group/...
and look at the various sub-topics, to see which functions really belong where.
Also... maybe not a novice project, but those Field array/object structures on the top-level topic are not relevant for Drupal 8 are they? That is probably a separate issue...
Comment #2
ThisIsDog commentedThere is no alternative field storage for Drupal 8. As a result there will be no Field Storage API section for 8. I went ahead through the Drupal 8 Field documentation functions and they seem to be sorted properly.
This seems like it would be a Drupal 7 documentation issue. I also don't see the array/object structures mentioned by jhodgdon in the current documentation, so I'm going to update the issue to Drupal 7.
Comment #3
ThisIsDog commentedComment #4
jhodgdonAgreed, this is now D7 only. Thanks for checking into this!
Comment #5
raulmuroc commentedComment #6
raulmuroc commented.
Comment #7
jhodgdonHi Raul, thanks for volunteering to take this on!
A few clarifications:
- hook_* functions should *not* be moved into .inc files. They should be left where they are. [hook_* functions are not really functions -- they are documentation with example function bodies -- and they need to be left in *.api.php files so that they are never loaded into Drupal.]
- To change the Topic/Group that a function is listed in, you need to make a documentation patch. The way that a function appears on a Topic page is one of the following:
a) In the function's documentation header, there could be a line saying
where topic_id is the machine name of the topic/group.
b) It could be between a
@defgroup topic_id @{and a@defgroup @}.c) It could be between a
@addtogroup topic_id @{and a@addtogroup @}.So in all cases, to move a function between topic_a and topic_b, you would need to change the topic_id in the *.api.php file.
In this case, the "Field Storage API" topic has ID "field_storage", and the other topics these belong in have other IDs.
Read more about @defgroup, @ingroup, and @addtogroup at
https://drupal.org/node/1354#defgroup
Hope that is enough information for you...
Comment #8
filijonka commentedComment #9
jhodgdonWell, that will probably work... The patch is a bit hard to review though (hard to tell if these functions that are moving are unmodified)...
How about if instead of changing the order of the functions in the .api.php file, you instead just move up the "end of addtogroup" and "end of defgroup" statements instead, so that functions are not being added to the topics/groups that aren't supposed to be?
And then you can just add one line to each of the functions that needs to be part of a different topic, like:
@ingroup foo
That will make the patch a lot easier to understand, help with git blame in the future, etc.
Comment #10
filijonka commentedHi
this was the approach I had with this
1. The functions weren't at the end of an @defgroup, so to just exclude (by moving enddef) that function wasn't a good solution, I would have had to changed several functions docheads.
2. The code should be easy to read and follow, if you wanna read about a defgroup functions you should be able to easily follow them in the file (incase you're not able to reach the api)
If I had choosed to do by 1 imo I would have to do so many other changes that isn't really necessary and also the readability of the file would have gone really bad. And I think it's more important of the readability in the file than in the patch but i'll take another look but anyone who feel for it is just to jump in.
Comment #11
filijonka commentedmust been an error that ingroup was used on field_attach so changed that to addtogroup
Comment #12
raulmuroc commentedComment #13
filijonka commentedsorry RaulMuroc, didn't see the assignment first time so made a patch but we still need a review of this so changing back to needs review
Comment #14
jhodgdonLooks pretty good! A few things to fix:
a) Syntax:
The @addtogroup field_types ... @{ needs to be in its own /** */ docblock, not merged in with the docs for the next hook.
Same with
down below.
b) I applied the patch, and looked through field.api.php to see if I thought all the hooks were in the right "group". The topics/groups we have are listed on the main "Field API" topic (@defgroup field -- https://api.drupal.org/api/drupal/modules!field!field.module/group/field/7 -- machine names of the topics in parens):
The hooks in field.api.php are, after this patch:
hook_field_extra_fields (and alter) - no group - should be in field_types perhaps?
hook_field_info (and alter) - field_types [good]
hook_field_schema (and alter) - field_types [good]
hook_field_* where * == load, prepare_view, validate, presave, insert, update, update_field, delete, delete_revision, is empty - field_types [good]
hook_field_prepare_translation - field_types - should this also be in field_language? (could add @ingroup to docs header)
hook_field_widget_* - field_widget [good]
hook_field_formatter_* - field_formatter [good]
hook_field_attach_* - field_attach [good]
hook_field_language_alter() - field_attach - shouldn't this be in field_language, or in addition?
hook_field_available_languages_alter() - field_attach - shouldn't this be in field_language, or in addition?
hook_field_storage_* - field_storage [good]
hook_field_info_max_weight - field_info [good]
hook_field_display_alter - field_types [good]
hook_field_display_ENTITY_TYPE_alter - field_types [good]
hook_field_extra_fields_display_alter - field_crud - should be in field_types instead, I think? field_crud is about actually deleting field types or detaching them from entities, not about reading field data from the database or displaying it.
hook_field_widget_properties_ENTITY_TYPE_alter - field_storage - should be field_widget I think?
hook_field_create_field/instance - field_crud [good]
hook_field_update_forbid - field_crud [good]
hook_field_update_field/instance - field_crud [good]
hook_field_delete_field/instance - field_crud [good]
hook_field_read_field/instance - field_crud [good]
hook_field_purge_field/instance - field_crud [good]
hook_field_storage_purge_field, purge_field_instance, and purge - field_crud - needs to be in field_storage also
hook_field_access - no group - should be in field_types I think
So I think we need to adjust just a little more. Thanks!
And by the way, I checked Drupal 8 just now, and there is only one hook there that is a problem. So rather than delay this on that one hook, I filed a separate issue:
#2260035: hook_field_info_max_weight in field.api.php is not in a group/topic
Comment #15
filijonka commentedIncase of not sure I used to add an ingroup so e.g the language that the review was uncertain about
hmm we have no file doc header of this..
Comment #16
filijonka commentedforgott to change status..
Comment #17
jhodgdonWhoops, sorry for not being clear!
This was correct before the patch. When you *define* a group with @defgroup, the rest of that doc block is the definition of the group/topic (summary line and paragraphs of docs). So that should be all in the same docblock.
It's when you are *adding* to a group with @addtogroup that you want to have two separate doc blocks. Sorry for the confusion! So that needs to be fixed in several places in the latest patch.
So... One more fix, and I think this is ready to go -- thanks!
Comment #18
filijonka commentedok np. perhaps update the doc about that so it's clearer #1354: [Obsolete] API documentation and comment standards?
Comment #19
raulmuroc commentedComment #20
filijonka commentedComment #21
jhodgdonRE #18 - https://drupal.org/node/1354#defgroup - it seems to show, in the examples, how to define a @defgroup, and how to use @addtogroup?
RE #20 - Looks excellent! Thanks for all the iterations!
Comment #22
jhodgdonThanks again! Committed to 7.x.