jforms, plugin block formcontrols: possibilité d'indiquer une liste de ctrl à afficher

[mike]

Suite à la discussion sur le forum http://jelix.org/forums/read.php?5,1071, prévoir la possiblité de passer une liste de contrôles à afficher au block {formcontrols}. Ceci permettrait d'afficher les contrôles comme on le souhaite. Ceci permet de completer la fonctionnalité du ticket #184.

Exemple d'utilisation:

{form $form ... }
{formcontrols array('login','email','ville')}
  <p><b>{ctrl_label}</b>{ctrl_control}</p>
{/formcontrols}
 
<table>
{formcontrols}
  <tr><td>{ctrl_label}</td><td>{ctrl_control}</td></tr>
{/formcontrols}
</table>
{/form}

[mike]

Le patch qui rajoute la fonctionnalité

[laurentj]

-    if(count($param) > 1){
+    if(count($param) > 2){
         $compiler->doError2('errors.tplplugin.block.bad.argument.number','formcontrols',1);

Il y a maintenant 2 arguments. Donc dans l'erreur, on doit indiquer 2, et non 1.

     if(count($param)){
-        $content = ' $t->_privateVars[\'__form\'] = '.$param[0].'; ';
+		    $content = '';

À priori, si on entre dans ce bloc if, il y a au moins un argument, donc normalement $content=' ' est inutile.

+				if($param[0] && !is_null($param[0]) && $param[0]!='null'){
+            $content .= ' $t->_privateVars[\'__form\'] = '.$param[0].'; ';
+				}

Dans le tableau $param, on ne reçoit que des chaines (representant le code PHP correspondant à ce qui a été écrit dans le template). Donc il est inutile de tester si c'est null ou pas. À mon avis, il faut faire le cas où count($param) == 1 et le cas où count($param) == 2.

Dans le premier cas où on n'a qu'un paramètre, il faudra générer du code PHP qui teste si ce que l'on reçoit est un objet jForms ou un tableau, et donc le code généré mettra la valeur dans $t->_privateVars['__form'] ou dans $t->_privateVars['__to_display_ctrl'].

dans le deuxième cas, pas d'ambiguité par contre, donc on peut tout de suite générer l'assignation.

+        if(count($param[1])){

Comme je dis précédement, on a des chaines dans $param, donc ce test n'est pas valide

+if(!in_array($ctrlref, $t->_privateVars[\'__displayed_ctrl\']) && (!isset($t->_privateVars[\'__to_display_ctrl\']) || (in_array($ctrlref, $t->_privateVars[\'__to_display_ctrl\'])))){

Pour les longs tests comme cela, vaut mieux ajouter des sauts de lignes.

Tu test aussi si to_display_ctrl existe, mais c'est inutile puisque 2 lignes avant tu le créé si il n'existe pas ;-)

+	$t->_privateVars[\'__displayed_ctrl\'][] = $ctrlref;

Ok, mais ce serait mieux de faire

 $t->_privateVars[\'__displayed_ctrl\'][$ctrlref] = true;

En effet, dans le test précédent, au lieu de faire un in_array, on ferait alors un isset, ce qui est plus rapide.

Le patch tel qu'il est ne sera pas inclus. Si tu pouvais le corriger suivant les indications, ce serait super. Corriger aussi l'indentation : dans jelix l'indentation se fait avec des espaces, et non pas avec le caractère tabulation.

[mike]

Nouvelle version

[mike]

Il y a maintenant 2 arguments. Donc dans l'erreur, on doit indiquer 2, et non 1.

Exact, j'avais pas vu l'argument en fin de ligne.

À priori, si on entre dans ce bloc if, il y a au moins un argument, donc normalement $content=' ' est inutile.

Ok, corrigé

Dans le tableau $param, on ne reçoit que des chaines (representant le code PHP correspondant à ce qui a été écrit dans le template). Donc il est inutile de tester si c'est null ou pas. À mon avis, il faut faire le cas où count($param) == 1 et le cas où count($param) == 2.

Effectivement cette solution est plus souple d'emploi.

Dans le premier cas où on n'a qu'un paramètre, il faudra générer du code PHP qui teste si ce que l'on reçoit est un objet jForms ou un tableau, et donc le code généré mettra la valeur dans $t->_privateVars__form? ou dans $t->_privateVars__to_display_ctrl?.

C'est ce que je viens de faire, mais je me demande s'il ne serait pas préférable de faire un éval pour connaître le type du paramètre, plutôt que de générer un code qui est testé à chaque fois (baisse des performances, code géneré plus lourd) Je ferais plus quelque chose comme ça:

if (eval('return is_array('.$param[0].');')){
    ...le code...
}
elseif(eval('return is_subclass_of('.$param[0].',\'jFormsBase\');')){
    ...le code 2 ...
}

Tu test aussi si to_display_ctrl existe, mais c'est inutile puisque 2 lignes avant tu le créé si il n'existe pas ;-)

Non c'est l'inverse je teste s'il n'existe pas dans le cas où l'on n'a pas passé de liste de contrôles en paramètres.

Ok, mais ce serait mieux de faire

 $t->_privateVars[\'__displayed_ctrl\'][$ctrlref] = true;

C'est vrai, c'est mieux je viens de le faire.

Le patch tel qu'il est ne sera pas inclus. Si tu pouvais le corriger suivant les indications, ce serait super. Corriger aussi l'indentation : dans jelix l'indentation se fait avec des espaces, et non pas avec le caractère tabulation.

J'espère que ces nouvelles modifs seront cette fois plus conforme à jelix pour être intégrées! Pour l'indentation je viens de modifier aussi (difficile de changer de norme de codage;)

[laurentj]

mais je me demande s'il ne serait pas préférable de faire un éval pour connaître le type du paramètre

Non parce que tu ne peux être sûr de ce que va donner cet eval. Par exemple, imagine que le paramètre soit '$form', ici il s'agit d'une variable, qui est transmise lors de l'affichage du template, elle n'existe pas lors de ton eval, donc le test va échouer. Idem quand tu donnes la liste des contrôles. On peut aussi bien faire un {formcontrols array(..)} qu'un {formcontrols $maliste}.

Par contre, on peut peut être, au lieu de faire ça :

elseif(is_subclass_of('.$param[0].',\'jFormsBase\')){

faire directement

else {

C'est moins robuste, mais ça évite un test.

Non c'est l'inverse je teste s'il n'existe pas dans le cas où l'on n'a pas passé de liste de contrôles en paramètres.

Arf oui, je me suis mélangé les pédales avec les noms des variables. En fait je crois que ton test dans le patch précédent était bon.

À part ça, ton patch est bon pour moi. Je vais juste changer les deux tests dont j'ai parlé.

Merci !

[mike]

Replying to laurentj:

À part ça, ton patch est bon pour moi. Je vais juste changer les deux tests dont j'ai parlé. Merci !

Parfait! heureux de pouvoir contribuer à jelix! Et sinon il sera intégré à quel moment dans le trunk?

[laurentj]

Patch intégré dans le trunk (500ième commit !!)