Help File Fixup: Comment module

Nice additions. A couple of comments,

I think the "About" section should have your first sentence. And then the rest of that paragraph should go under the "Uses," maybe with the dt, "Create comments." It's awkward that the dts under the "Uses" section, and especially the permissions one, are not really "uses." They relate to how you use comments, but not to their purpose, which the word "Uses" connotes. So I think it is important the first item in that list really be related to what you would use the module for.

Next point:

Permissions to access, post, post without approval, and administer comments are assigned per user role. When a comment has no replies, it remains editable by its author, as long as the user is authenticated.

I think writing in the active rather than the passive would be better. And since the second sentence here pretty much has to be passive, it would be nice to make the first active. So the first sentence might come out to be:

The comment module provides four permissions: access, post, post without approval, and administer comments. These permissions are assigned per user role.

As for the second sentence, maybe "logged in" instead of "authenticated." And also, it isn't directly related to permissions. It seems like it should have its own dt. So the dt might be, "Comments editable" with the dd being, "When a comment has no replies, it remains editable by its author, as long as the user is logged in.

Finally, maybe change second to last sentence to, ".... until a user who has permission to administer comments" or "who has Administer Comments permission." "... has Administer Comments" on its own sounds funny.

Shai

This is already so much easier to read! Here's a few suggestions:

1. I think that "For more information, see the online handbook entry for Comment module." should go at the bottom of the page to be more consistent with the other help pages. For comparison, look at the Filter or Taxonomy help pages.

2. Looks like you left off the array with the url link to content type. Here's your code:

    $output .= '<dd>' . t("Each <a href='@content-type'>content type</a> can have its own default comment settings configured as: <em>Open</em> to allow new comments, <em>Hidden</em> to hide existing comments and prevent new comments, or <em>Closed</em> to view existing comments, but prevent new comments. These defaults will apply to all new content created (changes to the settings on existing content must be done manually). Other comment settings can also be customized for each content type when you edit the content type's settings.") . '</dd>';

It should be:

    $output .= '<dd>' . t("Each <a href='@content-type'>content type</a> can have its own default comment settings configured as: <em>Open</em> to allow new comments, <em>Hidden</em> to hide existing comments and prevent new comments, or <em>Closed</em> to view existing comments, but prevent new comments. These defaults will apply to all new content created (changes to the settings on existing content must be done manually). Other comment settings can also be customized for each content type when you edit the content type's settings.", array('@content-type' => url('admin/structure/types'))) . '</dd>';

3. I find "they" in this sentence is a bit confusing.

When a comment has no replies, it remains editable by its author, as long as they have a user account and are logged in.

Can you change "they" to "the author"?

4. I think that the Drupal standard to write t(' instead of t(". The strings that start with "Each content type can have its own" and "Comments from users who do not have the" both use double quotes.

One small update to the patch is needed: the indentation is not consistent. It should only use spaces, not tabs.

I like the rest of the patch, though, so if the indentation is fixed up I would mark it RTBC.

Please double-check these changes in Garland.

Yeah, my comment on IRC was whether it really added anything to spell out the individual permissions here. I don't really think it does. We do that in node's help, but that's because these permissions are dynamically created whenever a new node type is added.

In other sections of the text, pointing out specific permissions where appropriate is well and good though.

One very minor detail: The post without approval permission is actually called "Post comments without approval".

Other than that, I like the current version.

The standard for capitalization is not being followed. See http://drupal.org/node/632280 -- should start with "The foo module" not "The Foo module" according to standard.

@jhodgdon: I don't see this being explicitly stated on the page you linked to, but the standard you want to apply here seems utterly wrong to me. "The Node module" refers to a proper name of a piece of software, and therefore, "Node" should be properly capitalized. I've seen many places in core where we use improper capitalization of module names. If necessary, we should create a separate issue to fix this bug in the documentation standards.

But no reason to hold up this patch.

The UX team approved the standard with "node module" uncapitalized. I'm assuming they checked it over carefully before they approved it. See discussion on #537828: Help text for core modules - update to conform to new standard. Please discuss with them before you decide the standard is wrong.

And we do need to figure out the standard before we patch all of these modules and then have to go back and patch them all again.

Committed to HEAD! Thanks! :)