This module, once the API is stable, will need to have a conditional_text.api.php file documenting the hooks that other modules can use to define plugins. Just filing this issue so it isn't forgotten; obviously it's a little early right now.

CommentFileSizeAuthor
#7 1191856.patch9.23 KBjhodgdon

Comments

jhodgdon’s picture

Priority: Normal » Major
tamasd’s picture

Status: Active » Needs review

Added in 8353f800463b852fc414d96de617f390fd405f12.

jhodgdon’s picture

This is an excellent start. I think it needs some work in a few areas.

a)

 * Condition plugins are made to handle the conditions. There are several
 * components.
 * There is a settings form, which can provide initial data for the plugin.

What are the other components? The only one I see here is the settings form, and the other parts are not explained.

b) Ideally, this file should be displayable using the API module. The part that starts:

/**
 * This is a display plugin.
 *
 * It controls how the conditional text will be displayed.
 */
$plugin = array(

will not work for the API module. It just can't handle this type of syntax -- a complex array just stuck into the file. So we need to figure out a different way to put this into the documentation. Usually, what we do is
@code
@endcode
within the @group block, and instead of using /** */ comments inside the plugin example, you would then just use // comments.

c)

 /**
   * The first syntax is when two functions are defined.
   *
   * In this case, the two theme functions has the following variables:
   * - text: This holds the conditional content.
   * - reason: The condition plugin can generate a reason text which explains
   *   why the text is hidden or shown.
   */
  'theme' => array(
    'false' => '',
    'true' => '',
  ),

This confused me. I think the 'false' => '' should be maybe 'false' => 'false_theme_function_name' or something like that? Then you should make a separate api.php entry for the sample theme function?

I also had the same problem with this one:

  /**
   * In this case, the complete output is handled by one theme function.
   *
   * This function has three variables:
   * - text: This holds the conditional content.
   * - reason: The condition plugin can generate a reason text which explains
   *   why the text is hidden or shown.
   * - result: This is the evaluation result.
   */
  'theme' => '',

d) The word "outputted" does not exist in English. Should just be "output".

e) So... After looking at this api.php file, I am still not sure how you would tell the Conditional Text module that your add-in module has defined a display or condition plugin. Maybe that is because I don't know about CTools, but I think it would be helpful to put something in the introduction about that.

f) I don't think that the part about how th parser parses the condition is necessary for this API file at all. Let's leave that out. What we want to provide is documentation that someone who wants to write a plugin would need.

jhodgdon’s picture

Status: Needs review » Needs work
jhodgdon’s picture

Actually, here's an idea.

Pretend you are a module developer coming to Conditional Text for the first time. Ask yourself: What are the steps to create a display plugin? What are the steps to create a condition plugin? And make sure the api.php file answers that question. Maybe instead of listing a sample for each, you could just point to the existing plugins as samples? If they are well-documented, then that would probably be good enough. You could just do something like this (actually, this is a more normal format in Drupal API documentation):

To define a display plugin, create an array in file myplugin.inc with the following components:
- title: The human-readable name of the plugin.
- description: The human-readable long description of the plugin.
- theme: An array of theme functions to use if the condition is TRUE/FALSE (indexed by 'true'/'false'). Omitting a theme function means.... The theme functions have the following arguments:
  (etc.)
tamasd’s picture

Version: » 7.x-1.x-dev
Assigned: Unassigned » tamasd
Status: Needs work » Needs review

New version: 4be1952fde5b1461d83ca95d4c09ffeea6de0832.

jhodgdon’s picture

StatusFileSize
new9.23 KB

This is really good!

There are a few small grammar errors, and some things that I thought could be clarified a bit...

And one other problem: The API module doesn't recognize comments like:

/**
 * stuff you are documenting
 */

unless they either contain @file, @defgroup, or come before a function, constant, etc.

So the sections you made:

/**
 * Display plugin.

and the other one for the conditional plugin, would not be displayed at all by the API module. They just need to be combined with the main @defgroup section.

Here's a patch to fix that problem and I think clarify the documentation a bit. Please make sure it's still accurate!

tamasd’s picture

Pushed: 82f1b4a6eb9bc68633eee2af12ace2a06513eec3.

Thanks for the patch!

I did some changes. In D7, every theme function has exactly one argument, called $variables, which holds the variables defined in hook_theme().

jhodgdon’s picture

Status: Needs review » Fixed

Looks good!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.