Admin help in core modules

Quick review:

1. We don't glue words together: %administerprofile should be %administer-profile, %administerwatchdogevents should be %administer-watchdog-events, etc.

2. We try not to abbreviate words: %stats should be %statistics.

3. We don't use spaces in a href-tags!
Wrong:
<a href = "%foo" title = "my foo">
Good:
<a href="%foo" title="my foo"%gt;

4. Why has this line been added: error_reporting(E_ERROR | E_PARSE);?

5. You can't write "administer >> menus >> add menu" without escaping the >>'s. In results in invalid XHTML.

6. Writing "administer >> menus >> add menu" is dangerous because these paths are dynamic (eg. they can be changed using the menu module). I guess it is uncommon though.

7. You changed settings.php.

8. We should avoid using the word "node" in user-level documentation. We try to use "post" or "content".

9. Some title-attributes of the a href-tag start with a capital letter, some with a small letter. Most title-attribute are useless; they don't add any value. In fact, they are often less descriptive than the content enclosed in the a href-tag. At least 90% of all title-attributes can be removed!

Examples:

<a href = "cron.php" title = "cron page">cron page</a> (redundant title)
<a href = "%adminsettings" title = "">administer >> settings</a> (empty title)
<a href = "%addavocabulary" title = "add a vocabulary">administer >> taxonomy >> add vocabulary</a> (useless title)
<a href = "%administertaxonomy" title = "administer taxonomy">administer >> taxonomy</a> (useless title)

Also, the patch only applies partially against HEAD. 6 modules don't get updated.

Quick review:

1. We don't glue words together: %administerprofile should be %administer-profile, %administerwatchdogevents should be %administer-watchdog-events, etc.

Ok, we can change that.

2. We try not to abbreviate words: %stats should be %statistics.

Ok, I'll change the Drupal handbook source for this.

3. We don't use spaces in a href-tags!
Wrong:

Good:

Ok, we can change that. What about closing tags?
4. Why has this line been added: error_reporting(E_ERROR | E_PARSE);?

cron page (redundant title)

redundant, but harmful?

administer >> settings (empty title)

Ok, we will find all the empty titles and either remove them or populate them with appropriate titles.

administer >> taxonomy >> add vocabulary (useless title)

The user base is changing. Simple english phrases are more meaningful to new users. I believe it's a mistake to overlook what is simple to the advanced user, but valuable to the new user.

administer >> taxonomy (useless title)

Again, a simple phrase while seemingly redundant to advanced users who are not intimidated by symbols such as >> can be helpful to new users, and those who rely on titles for accessibility. The hope was that themes that supported mouse over text would benefit over time as the documentation improved. However, it's a minor point and we can strip it away all the titles if it doesn't conform to Drupal guidelines and standards. I'd prefer to allow the documentation to evolve with more descriptive titles over time as documentation contributions increase.

We will aim to have a new patch meeting the points highlighted above in approximately the next 12 hours. We will also apply the same standards to the 30 contributed modules.

Thanks for the quick evaluation. It's greatly appreciated.

Cheers,
Kieran

Forum.module, locale.module, drupal.module etc. patches include some HTML errors, starting a new paragraph, then immediately starting another one. It might be a good idea to validate the HTML sources of the helps so random fixes in the future will not give more job to the translators. You can do this with extracting the po files, and examinging the files extracted. It is possible that given a standard HTML enclosure (html, head, body) the pot files will validate as XHTML, I never actually tried large scale help text validation.

There are also other HTML validity problems in the search module patch (unclosed list item tag). There might be others elsewhere.

It might also be a good idea to automatically spell check the strings (as it was done with the dev comments before), so that it is less likely that these need modification later. Keep in mind that translating all this stuff takes quite a bit of time (this is why these are not translated in some cases), so it would be good to do the best for the first time (which is your intention anyway :).

I think it would be nice if the help texts could be split up in one t()ed string per paragraph. That way they are much more likely to be at least partially translated by translators. Also, you can then leave the paragraph tags out from the t()ed strings and have a bit less html in those.

As a practicioning translator, and as the one who translated most of the long help texts in the complete Hungarian translation (and will do the updates :), I say having the text flow intact is more important then "helping" translators with breaking up text in smaller chunks. Having big chunks ensure that:

• That the string will not show up in the locale cache (because of their size), and will not slow down pages.
• It is easier to translate the complete texts, as you know you need to use the very same terminology around, and will not use random different translations, and you will also refrain from repeating the same "language salt", thus leading to quality translations
• Users of Drupal will not be faced with half translated help chunks (which is especially funny, if the middle of some English text is translated).

By the way, Drupal used to have per paragraph t(), but then changed to use one t(), which optimises database queries, and given the above I think it is better for the user and translator too.

Just my two cents...

Goba, I knew you would answer like that. ;)

I think that the single paragraphs are sufficiently selfcontained to translate them in pieces. I have looked at the aggregator help as an example.
You yourself are a kind of an übertranslator who'd also translate the whole bible in one PO chunk. ;)
So I think we should look at other translations, too.
I am using a very nice PO editor, but I know that I will never translate chunks of text that are more than two or threee sentence long. It takes too much time. But if I can split it up, I might translate the small pieces one by one. And I'd also be sure that I do not have to change everything just because a typo was fixed in the original.

The cache argument does not really count, because the pearagraphs will still be longer than 75 characters.
Also you already need to know where your translation will appear to do any Drupal translation. that is you will see the translated paragaphs in contex with the non-translated parts.

I agree that a half done itnerface looks funny, but its better than nothing.

Killes, your reasons are valid, but I still prefer the all-in-one approach. :) I will deal with it either way, so feel free to pick one way. I remember the transition from per-paragraph t()s to all-in-one t()s, and I think it was a good choice (and I have not heard much complaints, although I am not active enough in the translator community nowadays).

I firmly believed the reason for not getting complaints is that nobody even thought to translate those helptexts and is also a reason for not haveing many 100% translations.

There are a number of translations that are listed as being 100%, but that is at least partially due to the script which makes http://drupal.org/translation-status
It only counts the non-translated strings that are in the PO files in each directory, not the number of strings in the POT files.

The Ukrainian translation for example is listed as being 100%, but there are only 10 out of 33 in the repository. Guess I should fix the script.

Yes, it might be a good idea to msgmerge the po files with the actual pot files in the same CVS branch, and then check for completeness, so that a proper overview is available.

So, now what is the plan?

Per paragraph t()-ing or per helptext t()-ing?
I'll volunteer for making such a patch..

I am changing that to cvs.

My vote is obviously for splitting things up. PO editors were made to translate small pieces of text and not novels or even essays.

Then, so be it Killes!

I have to agree with you completely. Due to this (and inline with theme_placeholder($text), introduced lately by Steven/Dries), I'll come up with a patch which implements the helptexts, and introduce the following theme_* functions to get things a little easier and more drupalish:

function theme_text($text) {
  return "<p>$text</p>";
}

function theme_code($text) {
  return "$text";
}

This makes all the <p>-tags inside the helptexts redundant and helps us to clean up the texts a little more by removing a little more HTML out of it.. The same with the <code>

I am still thinking about how we should handle (un)ordered and definition lists, inside the helptexts.. Has someone another plan for this?

Please let me know what you think about this idea. I worked a lot on the helptexts before, and tried even mroe times to make them better and easier to translate. Due to that, I think this is the right thing todo..

But, maybe I am wrong..

I'll still volunteer for patching core/HEAD to get this in..

For now, let's not split each paragraph and let's not introduce new functions. Just fix up the patch as is.

Dries wrote,

"6. Writing "administer >> menus >> add menu" is dangerous because these paths are dynamic (eg. they can be changed using the menu module). I guess it is uncommon though."

I think we have to go with the assumption that if a user is changing menu paths then they should have some awareness that the path itself will no longer be valid, and as long as the paths are links, they'll still be able to access the correct pages.

Kieran wrote,

"9. . . . We were told to add titles for accessibility reasons. If this is not the case we can remove them."

See Dive Into Accessibility Adding titles to links:

"Not all links should have titles. If the link text is the name of an article, don't add a title; the link text itself is descriptive enough. But if you read the link text by itself, out of context, and can't figure out what it points to, add a title."

So the title must be more descriptive than the original link text in order to increase accessibility. One's that merely restate the original link text serve no purpose.

Charlie: let's turn that into an official documentation guideline?

" let's turn that into an official documentation guideline?"

Done. Updated documentation authoring guidelines 5a.

Never really used the help in Drupal, but going through this, looks really solid! Patch actually applies cleanly too, minus the settings.php included at the end ;) Looks good to me!

m3avrck: try to be more specific! What does 'looks good to me' mean? Does it mean you carefully studied and proof-read all changes, or that you merely skimmed over it. The former is helpful, the latter is not.

These patches, minus the settings.php, have been in CivicSpace for the last 2.5 months. If you want some end user feedback I am happy to get some.

WebChick and I are reviewing the comments on all the modules and improving the documentation by clarifying what's there, or creating child pages. If any one has content or links they would like to see added please let me know. We will aim to get a new patch to include the latest improvements in the admin help documentation. This patch will direct users to the handbook and so it is still accessible to them.

This is a nice feature to include in Drupal. It makes it a lot more usable if there is documentation that less tech savy users can consult when they are lost.

Having online documentation built in also is re-assuring to folks when being trained to use the code.

Also allows for more technical folks to pick up and start using the software without being trained.

Mike

ps. I haven't reviewed this specific patch but I have used the code in CivicSpace.

If the patch passes code review, I vote for getting it in as is. Rather than updating this patch further with new documentation, it would seem to be better to finalize it as necessary to get it in right now for HEAD. It's been months since this admin/help text update was started, and the existing documentation in this patch represents significant usability improvements. Seeing this new version of admin/help right now in 4.7 after the freeze will help to motivate more people to contribute assistance. We can incorporate any new changes in the documentation itself into an update for the official 4.7 release (since we need to do this anyway).

I'd like to note that this patch does not address Gerhard's concern about the legacy use of t()ed full admin/help texts as is currently in Drupal admin/help text and breaking them into individual paragraph t()ed strings. It is impractical to code these t()ed strings in paragraphs by hand. At the moment, the admin/help docs are autogenerated from a wide variety of scripts. As we will be submitting further updates for 4.7, the docs team would welcome a better method of generating the admin/help from the handbook that includes these suggestions for breaking up the admin/help text. Please see this issue for further information about admin/help generation script efforts in progress:

http://drupal.org/node/32078

Assuming that

* there are no other objections from those doing translations
* the patch works well with HEAD

I recommend that we go forward with this patch.

I'd think that grouping strings in a single t() function is pretty important for the quality of the translations.

Then the question is

1) Do we wait on this patch until someone steps forward to work with and modify the scripts that Kieran has to autogenerate the code? Then regenerate it? Reviews of the admin/help for 4.7 will wait until this patch is applied.
2) Do we move forward with the patch so that the doc team can more easily begin updating the admin/help for 4.7 by reviewing them from within Drupal? I've committed to reviewing and updating all of the core module admin/help within two weeks of the integration of this patch while others can begin working on the contributed modules. Then we'll wait a week or so to give others the opportunity to also review the updates I've made in the handbook to see If I've missed anything. So the roadmap is to have a new patch generated within a month so that the core admin/help is readily available before a release candidate. A volunteer could work on coding the script to parse the paragraphs, bulleted lists, etc. into separate t functions during that time. Assuming that the script is ready by then, we can include this in that update. If not, we can do another update once the script is created.
3) Scrap this patch all together. The doc team will begin working on updating the admin/help in the handbooks for 4.7 with the hopes that someone will volunteer to work on creating a script.

Personally, I'm very unenthused by (3) and will not be offering much of a commitment for quickly doing the updates to 4.7 in that instance for fear that we would end up right back here again. I'm pretty disappointed that the work this summer did not end up in the 4.6.3 release. I have 50+ teachers who could have benefited from the admin/help in their class sites this fall, and I can imagine hundreds of other Drupal users who could have done the same. I'd rather devote my time to other documentation work if my sense is that work on admin/help will continue to not benefit users.

OK, so I spend some time reviewing this patch (no one seems to really want to review it carefully enough):

1. '%book page content type' => url('book page content type') is not correct.

2. ,array should be , array (space). There are about 16 occurrences of this mistake. They need fixing.

3. a href = "http://drupal.org/handbook/modules/comment" title = "commentpage": there are spaces around the ='s, and the title isn't useful. Third, there is a space after the clode > tag. There are 40 occurences of these mistakes.

This is a bad patch. We have to clean it up. We cleaned up the sources at http://drupal.org/handbook/modules

We are working on tools to take the XML Docbook export and create the code which looks like this:

case 'admin/help#comment':
return t('

The comment module creates a discussion board for each post. Users can post comments to discuss a forum topic, weblog post, story, collaborative book page, etc. The ability to comment is an important part of involving members in a communtiy dialogue.

An administrator can give comment permissions to user groups, and users can (optionally) edit their last comment, assuming no others have been posted since. Attached to each comment board is a control panel for customizing the way that comments are displayed. Users can control the chronological ordering of posts (newest or oldest first) and the number of posts to display on each page. Comments behave like other user submissions. Filters, smileys and HTML that work in nodes will also work with comments. The comment module provides specific features to inform site members when new comments have been posted. On sites with active commenting from users, the administrator can turn over comment moderation to the community.

You can

• control access for various comment module functions through access permissions administer >> access control.
• administer comments administer >> comments >> configure.

For more information, read the configuration and customization handbook comment page.

',array('%admin-access' => url('admin/access'), '%admin-comment-configure' => url('admin/comment/configure')));
case 'admin/comment':
case 'admin/comment/new':
return t("

Below is a list of the latest comments posted to your site. Click on a subject to see the comment, the author's name to edit the author's user information , \"edit\" to modify the text, and \"delete\" to remove their submission.

");
case 'admin/comment/approval':
return t("

Below is a list of the comments posted to your site that need approval. To approve a comment, click on \"edit\" and then change its \"moderation status\" to Approved. Click on a subject to see the comment, the author's name to edit the author's user information, \"edit\" to modify the text, and \"delete\" to remove their submission.

");
case 'admin/comment/configure':
case 'admin/comment/configure/settings':
return t("

Comments can be attached to any node, and their settings are below. The display comes in two types: a \"flat list\" where everything is flush to the left side, and comments come in chronological order, and a \"threaded list\" where replies to other comments are placed immediately below and slightly indented, forming an outline. They also come in two styles: \"expanded\", where you see both the title and the contents, and \"collapsed\" where you only see the title. Preview comment forces a user to look at their comment by clicking on a \"Preview\" button before they can actually add the comment.

");
case 'admin/comment/configure/matrix':
return t("

Here you assign a value to each item in the comment moderation dropdown menu. This value is added to the vote total, which is then divided by the number of users who have voted and rounded off to the nearest integer.

• In order to use comment moderation, every text box on this page should be populated.
• You must assign the \"moderate comments\" permission to at least one role in order to use this page.
• Every box not filled in will have a value of zero, which will have the effect of lowering a comments overall score.

");
case 'admin/comment/configure/roles':
return t("

You can setup the initial vote value of a comment posted by each user role using these forms. This value is used before any other users vote on the comment. Blank entries are valued at zero.

");
case 'admin/comment/configure/thresholds':
return t("

Use these forms to setup the name and minimum \"cut off\" score to help your users hide comments they don't want to see. These thresholds appear in the user's comment control panel. Click \"edit threshold\" to modify the values of an already existing configuration. To delete a setting, \"edit\" it first, and then choose \"delete threshold\".

");
case 'admin/comment/configure/votes':
return t('

Create and control the possible comment moderation votes here. "Weight" lets you set the order of the drop down menu. Click "edit" to edit a current vote weight. To delete a name/weight combination go to the "edit" area. To delete a setting, "edit" it first, and then choose "delete vote".

');
case 'admin/modules#description':
return t('Allows users to comment on and discuss published content.');
}
}

Any help writing these tools would be greatly appreciated.

Kieran

Ok. Here is a brand spankin' new patch, generated from the PHP docbook parsing I've been crufting together over the past couple days (see: http://drupal.org/node/32078)

Features:
- Conforms to coding guidelines
- Contains current output from handbook
- Translatable text is broken up into chunks by paragraph/list item, rather than one huge blob (per killes)
- Handles external links, internal links (for instance, to cron.php), and normal everyday Drupal links

Please let me know what you think!

Sorry, i still don't like the output. I've reformmatted the aggregator help by hand and attach it for discussion.

Won't commit.

1. Don't break up help texts for now -- it doesn't work IMO. (See my previous comments and private discussion.)
2. Don't use $helptext, use $output. (We don't glue words together.)

Dries, why in hell do you put your non-translation experienced opinion before that of people actually doing translations? besides saying "won't work" you didn't even give an actual reason.

I have some translation experience, but not much. Mind you, I also wrote and maintained the first locale.module.

In Dutch, you can't properly translate 'administer' without knowing the 'you can':

+      $helptext .= '<p>'. t('You can') .'</p>';
+      $helptext .= '<ul>'. t('<li>administer localization at %admin-locale.</li>', array('%admin-locale' => l(t('administer >> localization'), 'admin/locale')));

Dries, please look at my changed aggregator help. Webchick will make her script to suport this syntax instead.
http://drupal.org/node/26139#comment-46644 This will resolve such problems.

OK. I reviewed webchick's patch, not yours.

BTW, no need to "why in hell" me.

Any updates?

OK, here is a new patch. Features:

1. Renamed $helptext to $output (per Dries)
2. Keeps the list items together in one big chunk, to retain context (per killes)
3. Adds link at the bottom of help text directing to module's Drupal handbook page for more information (per Amazon)
4. Parses out words in links >> like >> this and puts them into separate t() statements (per Stefan)
5. Figures out when it's the first time I've used $output and does $output = or $output .= accordingly (per Stefan)

The only difference between my formatting and the one killes suggested is I do:

      $output .= t('<p>You can</p>
<ul><li>administer your list of news feeds %admin-aggregator.</li>
<li>add a new feed %admin-aggregator-add-feed.</li>
<li>add a new category %admin-aggregator-add-category.</li>
...

rather than:

      $output .= t('<p>You can</p>
                  <ul><li>administer your list of news feeds %admin-aggregator.</li>
                      <li>add a new feed %admin-aggregator-add-feed.</li>
                      <li>add a new category %admin-aggregator-add-category.</li>
...

because the latter puts a bunch of spaces in the translation text rather than a single newline, and also looks quite silly when you view HTML source on a help page. Although it looks a bit funny in the module source code, the former makes things consistent as every line starts at the far left edge (I swear I saw a handbook page about not indenting inside t() as a best practice, but I can't seem to find it now).

Let me know what you think!

Didn't we agree previously to include the 'a href' tags in the documentation and to write:
t("... <a href="%url">foo >> bar</a> ...", array('%url' => 'baz'));
... and not to use t() in the arrays.

Also, you can't write '>>'. That would be invalid HTML.

I've had pretty extensive discussions with 3 or 4 translators, and was told to put ALL text in t()s, including link titles. The >> in the middle of t()s is not something that needs to be translated, therefore I was asked to take this out. Stefan Nagtegaal was the first one to suggest this, and I also confirmed with killes. (See http://drupal.org/node/32078 for the ongoing discussion about the script that actually generates this output).

Also, the >> gets translated by l() to >> when it's displayed. I initially put &gt;&gt; as the separator, but when it's displayed it turns into &ampgt; ;) So >> should be fine.

Ah, sorry. I misunderstood your first point. In other words, leave the link titles and HTML in the initial t() block and only move the URLs themselves out, because they shouldn't get translated. I'm not sure. But I can see how the current way could lose context for people (e.g. is that "administer" as a menu item, or "administer" as a verb, or..?) which is what we're trying to avoid.

killes, stefan, chx, Kobus..?

A classic example is "log in". If it is a verb, it should be "teken aan", otherwise it should be "aanteken". "user login" is less ambiguous in the sense that you can't confuse the two. In this case it would be better to say "gebruiker aanmelding" or even just "aanmelding" instead of "gebruiker aanteken", but basically, if you lose the context, your translation is not correct.

I am not exactly sure how t() works, so I can't comment on Dries' comment with accuracy. Does it translate everything except the %variables or does it translate nothing but the %variables or does it translate absolutely everything? If you put the tags inside the t() function, would it affect usability for users not HTML literate?

Then again, would a person that is totally HTML illiterate do translations? Where do you draw the line between usability and shooting translators in the foot? Take my example above. I'd rather see the whole string including tags (and duplicated data) if need be, rather than losing context.

I believe that if we can have some sort of structure when including the tags, e.g.:

t("<ul>
       <li>text 1 here</li>
       <li>text 2 here</li>
       <li>text 3 here</li>
       <li>%variable</li>
   </ul>", array('%variable' => 'text 4 here'));

Would be better than, say:

t("<ul><li>text 1 here</li><li>text 2 here</li><li>text 3 here</li><li>%variable</li></ul>", array('%variable' => 'text 4 here'));

which would be tough to edit, even for an HTML literate person.

My verdict (not technically justified) would be: include tags, but allow structure. I am not sure if that would be up to developers to do so, though, but I think it would be.

Regards,

Kobus

"I am not exactly sure how t() works, so I can't comment on Dries' comment with accuracy. Does it translate everything except the %variables or does it translate nothing but the %variables or does it translate absolutely everything?"

The variables in %variables are things that do not get translated; they get replaced with what follows after the t() statement.

For example:

"I like <a href="http://www.drupal.org/">Drupal</a>"

Would become:

t('I like <a href="%www-drupal-org">Drupal</a>', array('%www-drupal-org' => 'http://www.drupal.org/'));

Although currently it is:

t('I like %www-drupal-org', array('%www-drupal-org' => '<a href="http://www.drupal.org/">'. t('Drupal') .'</a>'));

Which means you get two translatable strings: the sentence itself, and then the actual link text. This was done to try and get HTML out of the string as much as possible, but I think it might make things more difficult because "Drupal" will have no context associated with it. However, it does mean that fewer strings are stored ("administer," "settings," and so on, for example, are words used many many times in all the help text pages).

HTML list markup is going to stay in the help texts, so they will not be HTML free. If we leave the link texts in there too, then we can get context for translation, which is very important. Hungarian has suffixes, and if you have no context, you cannot put in the correct suffix. Germans have der/die/das, and their variations, which change depending on what situation you use words (and the words themselfs also change).

Ok. I will roll a new patch later today Dries's way, with the title syntax. That will also reduce the size of my script by about 60 lines. *lol*

Ahem. With the t('blah blah <a href="%variable">link title</a> blah blah', array('%variable' => 'link/url'); syntax.

Thanks webchick.

Regarding the use of '>>'. Did you check whether the resulting help pages validate? Chances are you have to use the entities instead: '&gt;&gt;'. Please double-check.

Yep, they validate because l() does a check_plain() on them to translate them into entities (this is why escaping them as entities initially doesn't work, because it will escape the &). But the point is now moot because they'll be included in the text directly, so I will change them back into entities as they originally were in the text.

Will post an updated patch within the next couple hours.

Ok. One more try. Reformatted the links as discussed.

Kobus: Rather than:

t("

• text 1 here
• text 2 here
• text 3 here
• %variable

", array('%variable' => 'text 4 here'));

it is:

t("

• text 1 here
• text 2 here
• etc.

");

Hope that is ok. "Smart" indenting is beyond my regex abilities at the present time, and formatting the source documents like that by hand would take eons. :\

Argh!!

Rather than:

t("
  <ul>
    <li>text 1 here</li>
    <li>text 2 here</li>
    <li>text 3 here</li>
    <li>%variable</li>
  </ul>
", array('%variable' => 'text 4 here'));

it is:

t("<ul>
<li>text 1 here</li>
<li>text 2 here</li>
<li>text 3 here</li>
<li>%variable</li>
</ul>", array('%variable' => 'text 4 here'));

Hope that is ok.

Looks OK to me. It's not perfect, but it's committable. I'll leave it in the queue until someone else reviewed it.

I really hate to say this after 40+ hours I've put into this patch, but... :P

Please don't commit this yet.

I talked to Steef and chx on IRC, and think we may have come up with a good solution (though it will take some extra work):

1. Replace inline HTML with %tag and %-tag (so "Hello <em>World</em> becomes "Hello %em World %-em") -- not a perfect solution by any means, but does prevent translators from accidentally messing up HTML.
2. Use calls to the menu system to generate the breadcrumb nav to each menu item. This would mean:
a) No hard-coded >> in strings
b) No additional translation.. these menus are already translated

Will investigate more tonight and see what can be done.

Unless of course you want to commit the actual help text now, and we can work out the translation details later on.

Please, don't implement #1. It belongs in a separate patch. I'm not convinced this is a good approach. If a translator screws up a tag, then that is a bug like any other bug. It takes 2 minutes to fix. This feature sounds like cruft to me.

If we implement #2, make sure all help texts are updated. Consistency is key. #2 is certainly no requirement for me.

PS: is your documentation extractor script available in CVS? We have a directory in CVS for such scripts/snippets.

I am not convinced that the % notation for tags is any different then the normal HTML format for translators. In fact it is going to get confusing. Note that you inserted an extra whitespace before/after the em tag text, so that the tags don't mix with the text. This is normally solved in HTML with the angle brackets, and the extra whitespace might just confuse translators (it is not going to be displayed).

I'd like No.2 to happen, but I don't see how No. 1 would be an improvement. I am also not aware to any html bugs introduced by translators. Give them some credit. :p

Just a quick follow-up to this issue (sorry, I've been absolutely swamped).

I've uploaded the current script to http://cvs.drupal.org/viewcvs/drupal/contributions/tricks/docbook2helphook/. It is still a bit rough around the edges (eventually, I would ideally like to auto-retrieve the modules.xml file and also auto-update the module files), but it works.

No problem on skipping #1. It was my understanding that HTML presented a real issue for translators, but if that is not the case, then cool.

I've been looking into Drupal's menu-building stuff over the past couple days, but I don't see an immediately obvious way of giving a function a path name (ex: admin/settings) and having it spit out menu breadcrumbs for this (ex: administer >> settings). Am I missing such a function, or could someone give me a hint on how this could be accomplished using the functionality that's already there?

Neither #1 and #2 are requirements. We can move forward without the menu-stuff and worry about that later.

Talked with chx tonight, and he suggested the following (as far as I understood it anyway, feel free to correct me as it is quite late :)):

1. At the beginning of hook_help, store the current path. ($current_path = $_GET['q'];)
2. For each internal Drupal path that needs to be replaced, call menu_set_active_item($path) to change the current active item.
3. Then call menu_get_active_breadcrumb(), which will give you the "menu >> text" and set that as the replacement text. ('%admin-settings' => menu_get_active_breadcrumb())
4. At the end, restore *actual* current menu item. (menu_set_active_item($current_path);)

The problem there tough is that you can't do a foreach loop in the middle of an array declaration. So I believe this would have to involve the creation of another menu function which could take a Drupal path and run it through the breadcrumb rendering code. Then the rendered code coming from the docbook2helphook script could just be: '%admin-settings' => menu_get_breadcrumb('admin/settings'); (or whatever it was called)

Is it worth it to go there, or would the performance overhead of doing this kill any advantages we gain in making the way easier for translators?

> Neither #1 and #2 are requirements. We can move forward without the menu-stuff and worry about that later.

Oops, sorry. Didn't see this before I posted. :)

Sounds good to me. Anything else that needs to be done to make this commit-worthy?

Duplicate links in:

+', array('%elink-cron' => 'cron.php', '%elink-xmlrpc' => 'http://www.drupal.org/xmlrpc.php', '%elink-xmlrpc' => 'xmlrpc.php', '%admin-settings-drupal' => url('admin/settings/drupal')));

+', array('%admin-filters' => url('admin/filters'), '%admin-filters' => url('admin/filters')));

1. %elink-xmlrpc is used twice. Ditto for %admin-filters. Please investigate.
2. What is an 'elink'? Weird prefix. It can't mean 'external' because some 'elinks' are internal.

Please, focus on the documentation rather than on technical extra's. As I said, the patch is committable if the documentation is reviewed. Avoid playing menu system tricks at this point.

Consistency is still a problem:

+<ul>
+<li>Logs shows statistics for how many times your site and specific content on your site has been accessed.</li>
+<li>Referrers ells you from where visitors came from (referr url)</li>
+<li>Top pages shows you what the most popular content on your site is</li>
+<li>Top users shows you the most active users for your site</li>
+<li>Recent hits displays information about the latest activity on your site</li>
+<li>Node count displays the number of times a node has been accessed in the node\'s link section next to <em># comments</em>.</li>
+<li>Popular content block creates a block that can display the day\'s top viewed content, the all time top viewed content, and the last content viewed.</li>

1. Here, some sentences end with a . (point), others do not.
2. It's URL, not url.
3. 'referr' is a typo.

Same here:

+<li>administer user permissions at <a href="%admin-user-configure">administer &gt;&gt; user &gt;&gt; configure &gt;&gt; permissions</a></li>
+<li>administer content at <a href="%admin-node-configure">administer &gt;&gt; content types</a>.</li>
+<li>administer upload at <a href="%admin-settings">administer &gt;&gt; settings</a></li>

Regarding typos and minor inconsistencies. Should we address these now or in the revisions that we have to do to update this documentation to 4.7? All the documentation still has to be reviewed for changes between 4.6 and 4.7 and another patch submitted.

Ok. Let's try this one.

- Cleaned up the handbook in the areas you mention (fixed spelling errors on statistics module, updated each page to have a period at the end of their list item, misc. clean up, etc.)
- Changed prefix on files to be file- and external links to be external-
- Updated the script to remove duplicate entries for for replacement keys
- Put admin/help#module and admin/modules#description in a standard order across all modules

I've also modified docbook2helphook so it will actually attempt to patch the module files itself. There were a few issues with the initial convert (I had to do about 6-7 modules by hand), but overall it seems to work well and now that the help hooks are in order (there's *always* a case statement after the help hook), it should work great for the next big documentation switchover (*knock on wood* ;)).

Anyway, let me know what you think!

Just a very small update.

This also gets rid of all the remaining $section = "admin/help#module" arguments and makes them all just $section, for consistency.

Committed to HEAD! Great work.