Make exporting to module easier

I really like that idea! Big sry for letting it lie so long.

> If this is set your rules will be prefixed with the module name, and the #status will be set to 'deault'
I think we should not include that text. But instead let's explain that it generates default rules for a module. So I think we need the improve the texts a bit more. What about adding a small fieldset around the textfield with more help text in its description field? We should explain, that if used it creates default rules for module, then how one has to call the file and where to put it...

Furthermore the output of var_dump() isn't very nice and doesn't correspond to the drupal coding standards, that's odd. Perhaps we find an easy way to improve the code?

ok, here are my comments. It's quite a list though. However I really like this feature, so let's get it fixed.. ;)

1.)
+ $desc = '

'. t('Optional; You can export your rule and rule-set configuration to a file. This is better than having the rules in the database, as it allows having better control over your rules and their alterations.') .'

';
+ $desc .= '

'. t('Note that you rules are prefixed with the module name you provided - this will lower the risk of rule name collisions, and is considered a good practice.') .'

';

I'd not mention the collision stuff, the user shouldn't have to think about that. I'd suggest just using this help text:
"Optional: You can export your rule and rule-set configuration to a new module, which provides them as default configuration."

2.)
+ '#rows' => min($lines, 150),

I don't think such a big textarea is really easier to use? I think it's easier to have one small box, where you can easily copy/paste as it was before? What do you think?

3.) You are missing
- '#attributes' => array('readonly' => 'readonly'),

4) rules_item_type_invoke() shouldn't use $form_state. Better invent a variable that holds necessary information, like $module.

5.) This would be a good idea to do in both cases.
+ // Make code Drupal standard.
+ $code = substr_replace(str_replace("\n", "\n ", $code), '', -2, 2);

6.)
+ $output .= "description = Exports rules configuration of $module\n";
I think just
"Provides default rules configuration."
would be better.
Perhaps we should move the module in the rules group too?

7.)
+ // Change the rule set status to default.
+ if (!empty($form_state['values']['module'])) {
+ foreach ($export['rule_sets'] as &$value) {
+ $value['status'] = 'default';
+ }

You can and *should* do this directly in rules_item_rule_set_export(), this is what it is for.

8.)
+ if (!empty($form_state['values']['module']) && strpos($item_name, 'rules_') === 0) {
Better check if the rules name is already starting with the modules name, if not - add it.

9.) Instead of checkplaining the module's name everywhere, perhaps we should just restrict it to lowercase characters + underscores? So we enforce valid module names.

10.) I don't think you need to change the status for rules of a rule set when altering the set export, that's already done in rules_item_rule_export()

11.) Renaming Rule Sets! This is potentially dangerous, as configured actions to call the set wouldn't work any more. Best I think we try to fix this by doing a str_replace() over the whole export which fixes the export of such an action.

Instead of:

'Put this in @module.info in your modules/@module directory'

how about:

'Copy and paste the following code into @module/@module.info in your modules directory'

(pulled from features)

Why are we using my_module_rules_1 ? It should be just "my_module_1". Anyway I don't like the time(), it looks like an odd hack to me. Why not just re-number all rules to "my_module_X"? So what about just appending a simple character like "_2" as long until the name is unique?

Also what about integrating with the features module? I think this patch does exactly what features is for, so do you think we'd still need this patch if we'd do so?

@renaming rule-sets: Indeed you have not done so yet, however this is a problem. They also need to be prefixed with the module's name or we might ran into namespace conflicts :( -> see comment 11) from above.

>I think we should keep it as is, because here the user has control over the name of the rule set.

I don't think so. The user can only control the part after the 'rules_' prefix of the name to avoid name conflicts. Once a module provides a rule set it has to obey the naming conventions and prefix the rule sets, thus our provided module has to do so to.

> Also what about integrating with the features module?
I think we still need this patch, as Rules should allow one to export properly it's code without the need to rely on another module.

Hmm, I disagree! Sry, I know you have put considerable efforts into this patch, however I had a short look at the features module and I think it's the way for exporting. Consider a rule which uses flags, thus you want to be able to export this all together. Once flag also integrates with features, you can do so. Or think of rules using some content types + fields, just export theme using features!
However yes, rules should be able to export on its own - but it is already. The simple import/export like now of course stays, but I think for export to a module feature is the way to go and I don't think it makes sense to maintain two solutions for the same use case. So let's start looking into features integration. Again - sry for my late mind change - if hope you understand my intentions. Anyway I think we can reuse much of your patch for building the features integration!

Also perhaps you want to roll a separate small patch containing just improvements to existing export mechanism? Like removing the disabled property or obeying drupal code style?

>You can use hook_flag_default_flags()

Yep, but flags need to be exportable too and then they can have features integration.. :)

>You can use cck_sync module - Just showing you can use different ways for the same thing.

Yep, but doesn't it make sense to have all in one together just working? Ok, you need to enable a module for that, but this way you can always revert back to the module provided definitions (or even udpate it using features)...

>IMO feature can *compliment* the export not replace it (For example, Views gives this type of export to module as well)
Agreed, but I don't think we need to have module export on its own too, as soon as it's available via features!

-> #519954: integrate with the features module

Oh, it works that way. We have to write features integration that lives on top of the rules export. Also 1) is also no problem, exported features stuff also goes through hook_rules_defaults() - this hook won't change. So of course it's safe for og_vocab to do so. :)

ok, I've done some first refactoring of the export code so the features integration can build upon it. I've incorporated some of your improvements, thus I increased the size of the textarea and removed the readonly attribute. I didn't incorporate the indentation fix as I think this indention makes only sense when exporting to code.

Subscribing.

I'm sure Features will inevitably be the way to do this, but something that works independently would be nice...