Add optional extended help

I like the idea, though I don't think we should make an on/off option for the help. Ideally, would be always there and just one click away. E.g. it could be hidden by JS by default.

Still, we can surely add more help that is just visible by default. If its not longer than 2,3 sentences it should be fine.

Let's ping klausi about that.

New proposal:
Itangalo, So let's add more visible help (2-3 sentences) including points to the handbook for the advanced help?

I also think that the extensive help should be in the handbook pages. Adding some explanations here and there sounds good to me, but we should always link to the handbook pages. We could also hide the help in a javascript fieldset or behind a help symbol, if we think it clutters the page.

I don't think we should add links to screencasts in code, instead we could maybe create a Rules screencast learning page in the handbooks. Handbooks are easier to edit for people that are not familiar with the patch workflow.

+++ b/includes/rules.plugins.inc
@@ -542,7 +542,10 @@ class RulesLoop extends RulesActionContainer {
+      'description' => t('The list to loop over. The loop will step through each
+        item in the list, allowing further actions on them. See <a href="@url">
+        the online handbook</a> for more information on how to use loops.',
+        array('@url' => rules_external_help('loops'))),

Coding style says we should not add line breaks here. Just make a loooong line, editors can still break it for users.

+++ b/modules/system.eval.inc
@@ -195,6 +195,12 @@ class RulesTokenEvaluator extends RulesDataInputEvaluator {
+      '#description' => t('Note that tokens containing chained objects – such as

Let's call them "token replacements" not just tokens, because token == replacement is drupalism.

+++ b/modules/system.eval.inc
@@ -195,6 +195,12 @@ class RulesTokenEvaluator extends RulesDataInputEvaluator {
+        <em>data selection</em> input mode may help you find more complex

Yep, that sucks though is the best way to do it by now.
We should find a way to go with the token.module ui.

+++ b/rules.rules.inc
@@ -107,3 +107,68 @@ function rules_rules_evaluator_info() {
+ *   The key for a given topic. If given, the return will be the URL for the
+ *   topic online help. If omitted, the full array of external help pages will

It suffices to document this for @return once. $topic should be marked as " (optional) The key ..

+++ b/rules.rules.inc
@@ -107,3 +107,68 @@ function rules_rules_evaluator_info() {
+ *   be returned.
+ * @return

We should add a new line before return. I know it's violated in rules too, but let's start doing it right.

+++ b/rules.rules.inc
@@ -107,3 +107,68 @@ function rules_rules_evaluator_info() {
+      'url' => 'http://drupal.org/data-selection',

Can we do such nice urls? :D

+++ b/rules.rules.inc
@@ -107,3 +107,68 @@ function rules_rules_evaluator_info() {
+      'label' => t('Using complex tokens/replacement patterns'),

Let's say "token replacement patterns" ?

+++ b/rules.rules.inc
@@ -107,3 +107,68 @@ function rules_rules_evaluator_info() {
+    return url($help[$topic]['url']);

Why do we specify a label if we don't return it.

+++ b/rules.rules.inc
@@ -107,3 +107,68 @@ function rules_rules_evaluator_info() {
+  elseif (!is_null($topic)) {

Just use isset(), that's preferred to is_null.

+++ b/rules_admin/rules_admin.inc
@@ -29,6 +29,19 @@ function rules_admin_reaction_overview($form, &$form_state, $base_path) {
+    '#markup' => t('Reaction rules, listed below, reacts on selected events on

rules *react*

+++ b/rules_admin/rules_admin.inc
@@ -29,6 +29,19 @@ function rules_admin_reaction_overview($form, &$form_state, $base_path) {
+      may have any number of <em>conditions</em> that muste be met to execute

typo: muste

+++ b/rules_admin/rules_admin.inc
@@ -29,6 +29,19 @@ function rules_admin_reaction_overview($form, &$form_state, $base_path) {
+      these actions. You can also set up <a href="@url1">components</a> –

that must be met to fire the rule?
or
"that must be met for the actions to be executed"

+++ b/rules_admin/rules_admin.inc
@@ -95,6 +108,16 @@ function rules_admin_components_overview($form, &$form_state, $base_path) {
+      multiple places. You may also export each component separately. See

... and call it from your custom module? Is that of interest?

+++ b/rules_admin/rules_admin.module
@@ -90,6 +90,38 @@ function rules_admin_menu() {
+function rules_help($path, $arg) {

Why not just use rules_admin_help() ?

+++ b/rules_admin/rules_admin.module
@@ -90,6 +90,38 @@ function rules_admin_menu() {
+    include_once drupal_get_path('module', 'rules') . '/rules.rules.inc';

IT doesn't really fit in rules.rules.inc. Let's move it into ui/ui.forms.inc ?

That should be included every time the UI is used, we just have to include it here then.

+++ b/rules_admin/rules_admin.module
@@ -103,3 +135,19 @@ function rules_admin_form_alter(&$form, &$form_state, $form_id) {
+  if ($file->name == 'rules') {
+    $info['configure'] = 'admin/config/workflow/rules';
+  }

Oh that's nice - great idea!

+++ b/rules_scheduler/rules_scheduler.rules.inc
@@ -149,7 +151,13 @@ function rules_scheduler_action_schedule_validate(RulesPlugin $element) {
+    sure cron is configured correctly by checking your site\'s !status. The

There shouldl be no \' in t(). Use "" instead of ' and you can just go with "'".

+++ b/ui/ui.data.inc
@@ -65,6 +65,12 @@ class RulesDataUI {
+        available to Rules. <em>Note that entity fields will only be available
+        if you have used the condition \'entity has field\' (or \'content is of

Hm, that's not generally true. E.g. user account fields are always there, as there are no bundles. So maybe say if there are bundle-specific fields.. ?

+++ b/ui/ui.plugins.inc
@@ -180,7 +180,8 @@ class RulesLoopUI extends RulesActionContainerUI {
+      '#description' => t('The variable used for holding the each list item

the each..

>Because then the help topic will show up as "Rules UI" instead of "Rules", which I thought was bad.
I see, then we should comment that. :)

>The labels are used on the Rules help page. In a previous patch I had the help function add all the labels, but it seems much better to have them declared at the same place as the urls.
Oh, I see.

+++ b/rules_admin/rules_admin.inc
@@ -29,6 +29,13 @@ function rules_admin_reaction_overview($form, &$form_state, $base_path) {
+    '#markup' => t('Reaction rules, listed below, react on selected events on the site. Each reaction rule may fire any number of <em>actions</em>, and may have any number of <em>conditions</em> that must be met for the actions to be executed. You can also set up <a href="@url1">components</a> – stand-alone sets of rule configuration that can be used in Rules and other parts of your site. See <a href="@url2">the online documentation</a> for an introduction on how to use Rules.',

sets of Rules configuration, as it need not be a rule.

+++ b/rules_admin/rules_admin.inc
@@ -95,6 +102,11 @@ function rules_admin_components_overview($form, &$form_state, $base_path) {
+    '#markup' => t('Components are stand-alone Rules configuration that can be used by Rules and other modules on your site. Components are for example useful if you want to use the same conditions, actions or rules in multiple places, or call them from your custom module. You may also export each component separately. See <a href="@url2">the online documentation</a> for more information about how to use components.',

so let's use "sets of Rules configuration" again?

+++ b/rules_admin/rules_admin.module
@@ -90,6 +90,37 @@ function rules_admin_menu() {
+    // Sorry for returning a string of output rather than a renderable array.
+    // Apparently, this is how core wants it.

lol

+++ b/ui/ui.core.inc
@@ -1029,7 +1036,11 @@ class RulesConditionContainerUI extends RulesContainerPluginUI {
+          '#markup' => t('You are about to add a new @plugin to the @config-plugin %label. Any configuration indented under this plugin label will be a part of this plugin. See <a href="@url">the online documentation</a> for more information on condition sets.',

what does mean if a configuration is indented under a plugin?

Also, I think we should hide the "plugin" term from the user here, but use @plugin.

Then, configuration is in Rules usually the whole thing. The parts are called "elements".

I'll try to come up with an alternative. What about that?

"Use indentation to move a @plugin into the @config-plugin."

+++ b/ui/ui.data.inc
@@ -65,6 +65,8 @@ class RulesDataUI {
+      '#description' => t("The data selector helps you drill down into the data available to Rules. <em>Note that entity fields will only be available if you have used the condition 'entity has field' (or 'content is of type').</em> More useful tips about data selection is available in <a href='@url'>the online documentation</a>.",

Still, that's not true for user accounts. Should we ignore that?

+++ b/ui/ui.forms.inc
@@ -910,3 +910,70 @@ function rules_autocomplete_tags($tags_typed = '') {
+ * @param $topic
+ *   The internal key for a given topic.

What about calling this $topic_url ? The internal key for which to get a URL.

+++ b/ui/ui.forms.inc
@@ -910,3 +910,70 @@ function rules_autocomplete_tags($tags_typed = '') {
+ *   Either a url-checked URL for the given topic, or the full list of external

url-checked URL doesn't make much sense to me ;) Also, we don't need to run full url's through url() - it is already ready to use.

I just ran the tests with a failing test and I had no problems - so don't mind the testbot. It fails randomly for rules :( ui.forms.inc should be fine!

@rules_help()

I'm not sure about listing all that links there. It looks a bit random to me. Do you think this gives more value than just linking to the online documentation, where the links are anyway in a structured way? Maybe we should just add a short sentence what rules is about though.

hm, looks like there is an issue :/ I'll look into it.

>All comments in #27 now taken care of, except '$topic_url'. I see this key as the identifier of a help section, and the help section contains both a url and a label – so I still call it '$topic'.

The thing is that it is very strange that the topic definition has a label and a url, but you only get the url if you pass the topic.

To me it makes sense to have all links listed on the help page – it is a list of shortcuts to good Rules documentation, and it is also a way to find "that help link that was somewhere in the UI". However, I don't feel strongly for it, and if you think it's better with just a short help text and a general link I'm fine with that too. (That would also make the list of Rules help topics much easier, since each entry becomes a string rather than an array.)

Yep, that would simplify things a bit. I don't think its necessary, but I'll leave it up to you! Or maybe someone else has an opinion.

I think $topic is more descriptive than id though. I've shortly worked over the patch:
* Moved rules_help into rules
* moved rules_external_help into rules.module - now it's not long any more!

Please review.

Committed - just in time for 2.0 :)