Clean up API docs for dashboard module

Looks pretty good, thanks! A couple of things need fixing:

a)

  /**
-   * Send block order to the server, and expand previously disabled blocks.
+   * Sends block order to the server, and expand previously disabled blocks.

expand -> expands

b)

 /**
- * Dashboard page callback.
+ * Page callback: Display dashboard.

Display dashboard -> Displays the dashboard.

c)

  * @param $launch_customize
  *   Whether to launch in customization mode right away. TRUE or FALSE.
+ * @see dashboard_menu()

Leave a blank line before the @see.

d) dashboard_admin_blocks() needs @see for dashboard_menu(), and needs the line saying what path(s) it pertains to.

Also, looking through the dashboard.module file, there are a couple of other functions that haven't been fixed up and should be, such as:

/**
 * Ajax callback to show disabled blocks in the dashboard customization mode.
 */
function dashboard_show_disabled() {

What you did for the Ajax callbacks is probably fine. But if they are hook_menu() callbacks, they should also have the path?

Also, this needs a newline after the file docblock (sorry, probably didn't catch that last review):
+/**
+ * @file
+ * Ataches behaviors for dashboard module.
+ */
(function ($) {

And while we're at it, this could use "the":
+ * Adapts block's position to stay connected with mouse pointer.
Actually should probably be:
Adapts a block's position to stay connected with the mouse pointer.

And this could use "the":
+ * Page callback: Displays dashboard.

Thanks @sven.lauer! I noticed a few small things about the patch:

1. +++ b/core/modules/dashboard/dashboard.jsundefined
   @@ -1,3 +1,8 @@
   +/**
   + * @file
   + * Ataches behaviors for the Dashboard module.
   + */
   + ¶
   
   @@ -198,8 +201,10 @@ Drupal.behaviors.dashboard = {
   +   * Returns the current order of the blocks in each of the sortable regions.
   +   * ¶
   +   * @return
   +   *   The current order of the blocks, in query string format.
   

   Some trailing whitespace here.

2. +++ b/core/modules/dashboard/dashboard.jsundefined
   @@ -60,7 +65,7 @@ Drupal.behaviors.dashboard = {
      /**
   -   * Helper for enterCustomizeMode; sets up drag-and-drop and close button.
   +   * Sets up drag-and-drop and close button.
       */
   

   It might be good to make this a little more clear, maybe, "Sets up the drag-and-drop behavior and the 'close' button."

3. +++ b/core/modules/dashboard/dashboard.moduleundefined
   @@ -630,7 +645,7 @@ function theme_dashboard_region($variables) {
   - * Returns HTML for a set of disabled blocks, for display in dashboard customization mode.
   + * Returns HTML for disabled blocks, used in dashboard customization mode.
   
   @@ -652,7 +667,7 @@ function theme_dashboard_disabled_blocks($variables) {
   - * Returns HTML for a disabled block, for display in dashboard customization mode.
   + * Returns HTML for a disabled block, used in dashboard customization mode.
   

   These two are kinda comma splices. I'd say either make it two sentences, or (better) simply omit the comma and the word "used."

After applying the patch, I also found a few more things in the dashboard module that we can probably fix:

• DashboardBlocksTestCase does not have a docblock, and its test cases need the right verb tense in their summaries. Some are also two lines.
• Drupal.behaviors.dashboard could probably have it summary changed to "Implements..." rather than "Implementation of..."
• The CSS files could use @file blocks again.
• I'm not sure about this one because I've messed it up before, but in dashboard.api.php, should the hook summary begin "Add" rather than "Adds"? Reference: http://drupal.org/node/1354#hooks

Thanks for your work on all these!

Well, documentation standards are, by their nature, prescriptive, as is any formalized writing really. Different registers. :) I'd use constructions here in a comment that I would never use in the API documentation. Contractions, for instance. Incomplete sentences.

Edit: Also, as a note. Perhaps "comma splice" is just what the 8th grade English teachers call what is (to me) a comma that "sounds wrong." Those commas above sound wrong. That's not how a comma sounds.

Edit 2: I wonder if you would have had a less visceral reaction if I'd not called it a "comma splice"? Hmmm. Of course, none of this is on topic, but perhaps we can debate it some time. Nonsense up with which I will not put! ;)

Hmm, reading this:

Note that the change you are proposing "Returns HTML for disabled blocks, used in dashboard customization mode." => "Returns HTML for disabled blocks in dashboard customization mode." would actually change the meaning, if subtly.

I guess that's part of my discomfort with these two summaries. "Used" is ambiguous. Is the HTML used? The disabled blocks? The function itself? The comma does not denote any sort of relationship between "used" and a specific subject. I don't know what the intended meaning is.

However, we both agree it's a minor and (clearly) debatable point, so #9 is probably RTBC pending @jhodgdon's OK. ;)

Regarding comma splices and other usage/grammar/punctuation issues, on drupal.org we have some editorial standards for documentation, and we generally try to follow them in our API docs too.
http://drupal.org/style-guide/content

Basically, we use standard style references for stuff that isn't Drupal-specific. My style reference mentions comma splices as not being standard American English good style.

Anyway, whether or not we care about comma splices, I agree that the comma is not the best punctuation in those two examples mentioned in #8 item 3. I think we can do better at writing this documentation. For instance:

- * Returns HTML for a set of disabled blocks, for display in dashboard customization mode.
+ * Returns HTML for disabled blocks, used in dashboard customization mode.

How about:

Returns HTML for disabled blocks, for use in dashboard customization mode.

I think dashboard-rtl.css is still missing its @file block. Outside of that, this patch looks ready to me.

Ah, missed that too. I don't see the @file for dashboard.module in that patch, though?

Alrighty, everything looks correct now. Thanks @sven.lauer!

 <?php
-
 /**
  * @file

Do we really not want a blank line there (and other similar places in the patch)? Pretty much all of core has the blank line.

It makes more sense to me with one, since we always put blank lines before docblocks everywhere else, to separate them from what came before.

You mean at the very top of the file? Agreed, it looks like a lot of core has a blank line, but I don't have a strong opinion on whether there should or shouldn't be one. Does it add a lot to readability to have one there?

I think literally almost all of core has the blank line. In my patches I was removing a second line where there were two, but leaving when it was one. I guess I think it looks a little nicer with the blank line, but mostly it's probably best to be consistent.

I bow to your greater wisdom. :)

Pffffff. :P

Here's a reroll of @sven.lauer's patch that simply restores the blank line in PHP files.

okeydokey!

+ * @file
+ * Provides a dashboard page in the adminstrative interface.

Sorry, just noticed this ("adminstrative" => "administrative"). I'd reroll but then I'd get commit credit for a patch I didn't actually do anything meaningful for :)

Well, I know webchick at least prunes rerollers and other random-attachment-adders from the commit mentions. Dries, however, just uses whatever dreditor says. Not sure about catch. ;)

Since I already rerolled once, I'll take the risk!

Right, yeah, that second isn't a patch. ;)

I need to stop proofreading. (See the attached interdiff.) Now I too can be added to the commit list, I guess...

With this little fix I think it's good to go.

Actually, "administrator-only" doesn't really make sense. There is nothing about those blocks that make them only for administrators.

How about "administrator-flagged" instead?

Now I can say I've actually contributed to this patch... sort of :)

Maybe just "administrative"?

Definitely seems reasonable. Since that's the only change I'm putting this back to RTBC.

#34: dashboard-1332658-34.patch queued for re-testing.

Stripping "Paths: " lines per #1315992: No standard for documenting menu callbacks.

Committed to 8.x. Moving to 7.x.

I checked #36 for the "new" standards, and while there are several changes to summaries that add "Foo callback:" prefixes, in all cases it's just a reordering of words already there that makes the summaries follow our general standards.

D7 backport.

Thanks @Albert Volkman! Everything there looks to be okay to backport, except for these parts:

+++ b/modules/dashboard/dashboard.moduleundefined
@@ -263,10 +267,12 @@ function dashboard_forms() {
+ *
+ * @see dashboard_menu()

@@ -298,11 +304,12 @@ function dashboard_admin($launch_customize = FALSE) {
  *
+ * @see dashboard_menu()

@@ -485,7 +492,9 @@ function dashboard_dashboard_regions() {
+ *
+ * @see dashboard_menu()

@@ -506,12 +515,14 @@ function dashboard_show_disabled() {
+ *
+ * @see dashboard_menu()

We need to not add these @see to the menu hook implementation in D7. (As I stated above, though, I think the changes to the summaries of those same callbacks are fine because they are really just reordering words that are already there.)

@see dashboard_menu() removed!

Looks good to go now; thanks!

All of this makes sense except for:

- * Adds regions to the dashboard.
+ * Add regions to the dashboard.

I thought we always put S on the end of the first word of the function descriptor.

I thought hook documentation does it the other way, without the "s" (although I'm not entirely sure why).

#46 is correct. Hook documentation is not the same as others, because we're documenting "If you want your module to do X, implement this hook", not "This function does this action".

Oh bloody hell. :)

Ok, committed and pushed to 7.x. Thanks.