Clean up API docs for taxonomy module

Do you plan to patch this module? If so can you assign the issue to yourself and also add your name to the summary issue? Thanks!

The patch for this issue should include correcting the docblock for taxonomy_autocomplete() as described in #992020-25: Taxonomy autocomplete does not respect cursor position..

Not anymore. :)

I know taxonomy.module pretty well and I also know what issues the patch is likely to collide with. Tentatively assigning this one to myself for that reason. Going to wait for includes A-C to land before I start, though.

tagging

Just waiting on the includes issues to get in before I do Taxonomy.

Updated issue summary.

I'm going through these issues and un-assigning any without activity for several weeks. If you still want to work on this issue, please feel free to assign it back to yourself. Thanks!

I need something that's not too demanding to work on between meetings today, so I'm going to look at this issue again. In the spirit of keeping the scope narrow, I'm going to start with just the first three points from the summary, since that should be really easy to rebase around.

Turns out there weren't that many formatting errors, so I also took care of rewrapping/rewriting the few lines that were over 80 characters. I'd say do the summaries separately, if that's cool with @jhodgdon.

+++ b/core/modules/taxonomy/taxonomy.admin.incundefined
@@ -518,7 +518,7 @@ function theme_taxonomy_overview_terms($variables) {
- * Returns a rendered edit form to create a new term associated to the given vocabulary.
+ * Renders an edit form to create a new term in a given vocabulary.
  */
 function taxonomy_term_add($vocabulary) {

The only questinable hunk. Function returns build array so "renders" probably better to replace with "builds"

Thanks @andypost and @Lars Toomre.

All of the changes in the patch are appropriate.

The only thing I noticed is a very small nitpick: 'An array ' line needs one more space before 'An ...'.

That line is part of the diff context, not part of the changed lines, so it's not in scope. See: http://webchick.net/please-stop-eating-baby-kittens

#11 is correct though; I agree that "Builds" is better.

The patch has a couple of different sections with the same files in them?

Patch #15 works fine to me.

Rerolled patch with this changes:
-Changed taxonomy-term.tpl.php location. From core/modules/taxonomy/ to core/modules/taxonomy/templates

@Lars yes, I did check all changes you made, line by line.

I do not understand what do you mean about tpl location. Patch in #15 failed to apply due to wrong path, and that's what I wanted to fix (which seems I have failed -it's my first patch and first drupal core contrib).

RE #23 - just file separate issues for hooks that need better documentation. Thanks!

This looks overall pretty good, thanks!

A few things to clean up:

a) We don't normally include @param $form in form constructors [taxonomy_overview_vocabularies()]

b) In-code comments should be left out of these patches and done on the Coder Review issues instead. And definitely any other code formatting and code line changes do not belong in this patch.

c) When creating @return docs, we don't normally include "Returns" in the documentation, such as here:

+ * @return array
+ *   Returns the built and processed term form array.

Should just say "The built and processed..."

d) All the first-line changes to verb tense in hook docs in taxonomy.api.php are incorrect, see
http://drupal.org/node/1354#hooks

e) In taxonomy.module:

/**
- * Users can create new terms in a free-tagging vocabulary when
- * submitting a taxonomy_autocomplete_widget. We store a term object
- * whose tid is 'autocreate' as a field data item during widget
- * validation and then actually create the term if/when that field
- * data item makes it to taxonomy_field_insert/update().
+ * Users can create new terms in a free-tagging vocabulary when submitting a
+ * taxonomy_autocomplete_widget. We store a term object whose tid is
+ * 'autocreate' as a field data item during widget validation and then actually
+ * create the term if/when that field data item makes it to
+ * taxonomy_field_insert/update().
  */
 

This doc block is not documenting anything. The text here should be moved to some doc block that is actually documenting something: a defgroup, the @file block, a function, or ... ??? but it is not doing anything useful here.

f) Punctuation around e.g. -- should always be "blah blah blah; e.g., a, b, or c.".

g)

* @param Drupal\taxonomy\Term $term
  *   A taxonomy term entity.
+ *
+ * @return bool
+ *   TRUE if the current page is for the passed in term ID; otherwise, FALSE.
  */
 function taxonomy_term_is_page(Term $term) {

The passed-in value is the term object, not the term ID, so this @return doesn't quite make sense?

h)

 /**
- * Find all ancestors of a given term ID.
+ * Finds all ancestors of a given term ID.
+ *
+ * @param int $tid
+ *   A taxonomy term ID.
+ *
+ * @return array
+ *   All ancestor terms, if any, for a given term ID.
  */
 function taxonomy_term_load_parents_all($tid) {

@return ... should be "the given term ID" at the end... and what are "ancestor terms" -- are they term IDs, term objects? Maybe should also start with "An array containing..."?

i)

+ * @param array $tags
+ *   An array of vocabulary term tags.
+ * @param int $vid
+ *   (optional) A vocabulary ID to constrain terms to. Defaults to NULL.
+ *
+ * @return string
+ *   A string with a comma-delimted imploded list of tags.
+ *
  * @see drupal_explode_tags()
  */
 function taxonomy_implode_tags($tags, $vid = NULL) {

$tags -- is it an array of term IDs, term objects, or what?

j)

* Possible error codes:
- * - 'taxonomy_term_illegal_value': The value is not part of the list of allowed values.
+ * - taxonomy_term_illegal_value: The value is not part of the list of allowed
+ *   values.
  */
 function taxonomy_field_validate($entity_type, $entity, $field, $instance, $langcode, $items, &$errors) {

I don't think we want to remove the '' here. This is I think a literal string?

Re-roll of #22 against current head. Still needs fixes from comment #26.

Updated patch with comments from #26.

Re-roll.

There are still many problems with this patch:

a) One line of my pet peeve of "ids" instead of "IDs" in here:

 * @param $ids
- *   An array of ids to reset in entity controller cache.
+ *   (optional) An array of ids to reset in entity controller cache. Defaults to
+ *   NULL.
  */
 function taxonomy_vocabulary_static_reset(array $ids = NULL) {

b) This change to taxonomy_get_children() is also wrong:

 * @param $vid
- *   An optional vocabulary ID to restrict the child search.
+ *   (optional) An vocabulary ID to restrict the child search. Defaults to zero.

"An vocabulary" => "A vocabulary". And if you are going to say what it defaults to here, saying it defaults to zero is useless because obviously zero is not a valid vocab ID. Either leave that out or explain what it means; preferably leave it out, since saying it's optional implies that if you don't provide it, there is no restriction, right? All of this "defaults to NULL" stuff is really dumb when it provides no information and drives me crazy!

c) Here's another one:

+ *   (optional) The number of levels of the tree to return. Leave NULL to
+ *   return all levels. Defaults to NULL.

It says "leave NULL to..." so "Defaults to NULL" is just redundant. Waste of typing and space and bytes to include this stuff.

d) And another one where it just makes you go "huh?":

 * @param array $tids
  *   (optional) An array of entity IDs. If omitted, all entities are loaded.
+ *   Defaults to NULL.

If omitted... defaults to NULL? What? This occurs several times. IMO "Defaults to NULL" here is not only redundant but confusing.

e)

+ * @see Drupal\Core\Entity\EntityFieldQuery
+ * @see entity_load_multiple()
  */
 function taxonomy_term_load_multiple(array $tids = NULL) {

If you use a namespace in docs, it MUST start with \

Got rid of the "Default to NULL" stuff, fixed namespace docs.

Removed "Needs reroll" because requested changes have been included in the latest patch.

Updated issue summary.

Did an iteration on this. I vastly underestimated the amount of work that was left, continued until I ran out of steam. There is still a lot of work to be done. Especially the Views integration has very poor quality documentation.

I also fixed some whitespace problems which I noticed, hopefully I did not step out of the issue scope with those.

I pondered on this for a while but couldn't come up with a fitting description that fits in 80 characters, so some help from a native English speaker is welcome:

/**
 * Change {taxonomy_term_data}.vid into a string holding the vocabulary machine name.
 */
function taxonomy_update_8006() {}

I really think we should just drop all of these "clean up the API docs" issues. They are nearly impossible to get right and require a huge amount of time to review. Sorry... I've said this before... let's really just close this?

I'm ok with that, this almost caused me to never want to touch documentation cleanup issues again ;)

OK then...