developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 12 years ago

Closed 12 years ago

#317 closed bug (fixed)

les fichiers de controller ne peuvent avoir d'underscore dans leur nom

Reported by: bastnic Owned by: laurentj
Priority: highest Milestone: Jelix 1.0 RC1
Component: jelix:core Version: 1.0 beta 3.1
Severity: critical Keywords:
Cc: Blocked By:
Blocking: Documentation needed:
Hosting Provider: Php version:

Description

Sinon par exemple pour le fichier toto_titi.class.php il cherche la méthode default dans le fichier toto et non toto_titi

Attachments (4)

patch_without_defaultaction_feature.diff (6.0 KB) - added by nuks 12 years ago.
Voir toutes les explications dans le commentaire spécifique.
full_patch.diff (6.3 KB) - added by nuks 12 years ago.
True leader !
full_patch3.diff (17.4 KB) - added by nuks 12 years ago.
full_patch4.diff (148.7 KB) - added by laurentj 12 years ago.
Nouvelle version du patch, plus complète

Download all attachments as: .zip

Change History (26)

comment:1 follow-up: Changed 12 years ago by laurentj

  • Component changed from jelix to jelix:core
  • Milestone set to Jelix 1.0 RC1
  • Owner set to laurentj
  • Priority changed from normal to highest
  • Severity changed from normal to critical

Si on veut pouvoir mettre des _ dans un nom de contrôleur, il va falloir changer la syntaxe des sélecteurs d'action : remplacer le _ qui sert à séparer le controleur de la méthode par un autre caractère ( ":" par exemple).

Le souci de changer est que l'on ne pourra pas prendre en charge à la fois les anciens sélecteurs et les "nouveaux" sélecteurs. Un boolean dans le fichier de config pourrait être implémenté pour dire si on utilise les anciens ou pas, pour pouvoir migrer son appli vers jelix 1.0 sans avoir à tout changer.

Voir maintenant quel nouveau caractère on choisi. Il faut qu'il puisse être utilisé dans une url sans qui faille l'encoder, que l'on puisse l'utiliser dans les noms des methodes en xml-rpc, json-rpc. À priori, ":" semble aproprié, et rappelle la notation PHP classe::methode();.

comment:2 Changed 12 years ago by bibo

Alors pourquoi pas '::' carrément.

comment:3 Changed 12 years ago by nuks

  • Owner changed from laurentj to nuks

Je fais un essai.

comment:4 in reply to: ↑ 1 ; follow-up: Changed 12 years ago by nuks

Replying to laurentj:

Voir maintenant quel nouveau caractère on choisi. Il faut qu'il puisse être utilisé dans une url sans qui faille l'encoder, que l'on puisse l'utiliser dans les noms des methodes en xml-rpc, json-rpc. À priori, ":" semble aproprié, et rappelle la notation PHP classe::methode();.

Mmmmh, j'ai presque fini, le problème est que le caractère : est encodé dans l'url. En gros, j'ai une url comme celle ci : http://localhost/index?module=devsite&action=default%3Aindex

Ps : deuxième bug, ton fichier INSTALL pour testapp a été renommé en INSTALL.txt a cause des système insensible a la casse (et d'un conflit avec le dossier install). Le fichier testapp.mn n'à pas édité, je l'ai fait, si tu as la patiente de vérifié mon patch.

Par contre il faudrait adapté tout les testes unitaires en conséquence pour le double point :/

Changed 12 years ago by nuks

Voir toutes les explications dans le commentaire spécifique.

comment:5 in reply to: ↑ 4 Changed 12 years ago by nuks

Le patch est fonctionnel, mais il y a un nouveau un problème quand il n'y a pas de controleur par défaut dans urls.xml pour le moteur d'url significant. Je vais essayer d'arranger ça. J'ai aussi arranger la commande de compilation buildapp comme noté plus haut. Les testes unitaires n'ont pas été effectués : Il va falloir tout rechanger pour accepter le double point, ça va prendre une plombe !

Je vais essayer d'arranger le problème du controleur et je republierai le patch.

comment:6 Changed 12 years ago by nuks

Bon eh bien voilà :) J'ai résolu le problème du défaut dans urls.xml par une manière très simple (Remplacer "%3A" par ":"). Il doit y avoir une manière plus propre de le faire, mais c'est fonctionnel et je n'ai pas trouver d'autre solution.

Pour le tester, n'oubliez pas de mettre la variable oldControllerNaming à off et de changer tout les séparateurs de vos action/controleur, y compris dans le fichier config.ini.php.

Ps : laurentj. Je pense pouvoir mettre au point un système permettant d'utiliser les deux séparateurs : il suffit de regarder si le sélecteur contient un ":". Si il n'y en a pas, le système se base sur le séparateur "_".

Pour les testes unitaires, c'est toujours à faire...

Changed 12 years ago by nuks

True leader !

comment:7 Changed 12 years ago by laurentj

Super pour le patch ! Mais il n'est pas encore complet et il y a encore des trucs à corriger.

  • Supprimer la modif dans testapp.mn, elle a été faite par bballizlife dans le trunk
  • Je pense qu'il faut choisir un nom plus adéquate pour l'option dans le fichier de config. Je propose plutôt oldActionSelector.
  • respecter le style de codage, en particulier au niveau des accolades : l'accolade ouvrante ne passe pas à la ligne.
  • Pour l'encodage du ":" dans l'url : l'emplacement de ta correction m'etonne, puisqu'il s'agit de la compilation du fichier urls.xml. Cela voudrait dire que lors de la lecture du fichier xml, php encode le ":" ? Bizarre. Par contre, là où je pense il faut corriger, c'est dans jUrl, aprés les fonctions http_build_query, qui encodent effectivement le ":" alors qu'elles ne devraient pas, conformément à la rfc 3986. (malgré que de plus anciennes RFC comme la 1738 définissait le ":" comme caractère réservé dans la partie query d'une URL). Et il ne faudra retransformer le %3A en ":" que sur le résultat de http_build_query. Il y a aussi à faire le remplacement dans les moteurs d'url au niveau du parsing.
  • dans jSelectorAct, au niveau de la regexp, tu ne tiens pas compte de oldControllerNaming. En fait, partout où il faut faire une modif pour tenir compte de ":", il faut tenir compte de oldControllerNaming
  • Il y a d'autres endroits à corriger, comme dans jCoordinator::process(), ou peut être aussi le moteur d'url simple. Bref, il va falloir vérifier toutes les classes du core, les plugins et autre.
  • Pour les tests unitaires : il faudra garder les tests actuels pour continuer à tester l'ancienne syntaxe, mais bien sûr il faudra ajouter les mêmes tests pour la nouvelle syntaxe.

Si tu as le temps de faire tout ça, dis moi sinon je le fais.

comment:8 follow-up: Changed 12 years ago by laurentj

Ah oui, et il y a aussi l'extension en C à modifier, mais ça je m'en occupe :-)

comment:9 Changed 12 years ago by nuks

Je vais essayer de te faire tout ça :p

Modif supprimée. Accolades corrigées. Nom de variable changé.

Pour l'encodage du :, je ne sais plus comment j'en suis arrivé à le mettre là, mais c'était après une série de teste, je vais corrigé tout de suite ça, je sais quoi faire. Tu as raison, c'est bien http_build_query qui foire dans jUrl.

Pour jSelectorAct, là je ne vois pas de quoi tu parles. Il y a un truc avec un pre-processeur où j'ai pas vraiment chipoté. Il y a bien une variable $regexp, mais elle se trouve dans jSignificantUrlsCompiler. Si c'est de celle là que tu parle, je n'y ai pas touché car elle concerne le pathinfo et non l'action. Si tu parles des regex de jSelectorAct(Fast), je les ai bien adapté :/

Pour le moteur d'url simple, je ne pense pas qu'il faille y toucher :/ En effet, on ne split même pas action pour avoir le controleur et l'action. Quand à jCoordinator::process(), tu as raison ;) Je vais tout passer au peigne fin :/

Pour les testes unitaires, j'ai pas trop le temps pour le moment :| Je commence les exams mardi et j'ai a peine été voir le code source pour corriger le teste avec le "default_" du moteur d'url significant.

PS : J'ai remarqué que certaine source utilisait 3 caractères pour l'indentation. Mon IDE va peut-être tout corrigé automatiquement dans les sources ou je vais voire donc le patch aura peut-être une taille un peut grosse :) PS2 : Après tout ça je pense pouvoir adapté facilement le code pour qu'il acceptes les deux séparateurs. J'ai déjà fait quelques testes et c'est faisable mais il y a un problème. Quand on utilise la notation "_" dans urls.xml et qu'on fait un appel avec la notation ":" par exemple, ça ne fonctionne pas. Ca ne doit pas être difficile de s'arranger pour transformer le tout les "_" en ":" qui passent dans l'engine url significant et dans la classe jUrl, mais ça risque de ralentir un peu le code. J'attend ton avis.

comment:10 in reply to: ↑ 8 Changed 12 years ago by nuks

Bon, j'ai vérifié tout. Je ne suis pas sure d'être à 100% complet. De plus, il y a un endroit où je ne suis pas certain : La classjXmlRpcRequest contient un explode(':', $nom). Regarde ce que j'ai fais de de ce côté là pour vérifié stp.

Il va falloir attendre les testes unitaires.

Changed 12 years ago by nuks

comment:11 Changed 12 years ago by nuks

Ah oui, j'oubliais. Pour les templates j'ai utilisé une syntaxe comme ceci à l'intérieur des templates : {if $GLOBALSgJConfig?->oldActionSelector == false} Je ne suis pas sure que c'est fonctionnel, mais sinon, il y a toujours moyen de faire 2 fichiers templates.

comment:12 Changed 12 years ago by laurentj

  • Owner changed from nuks to laurentj
  • Status changed from new to assigned

Si tu parles des regex de jSelectorAct(Fast), je les ai bien adapté :/

Oui je parlais de ça, mais oublie, c'est moi qui ai lu trop vite.

Sinon le patch me parait bien. Je ferais les tests unitaires et completerais le patch. Va bosser pour tes examens :-)

PS : l'indentation officielle est de 4 espaces. Là où il y a 3 espaces, c'est une ancienne guideline et j'ai pas tout converti encore :-).

comment:13 Changed 12 years ago by nuks

Pas de problème. Juste une question, le "owner" est le propriétaire du ticket, ou celui qui le résous sur le moment ?

comment:14 Changed 12 years ago by laurentj

À priori, le owner c'était toi au début. Mais comme je prend la suite, c'est moi maintenant. (non tu t'arretes là, va travailler tes examens ! :-) )

comment:15 Changed 12 years ago by nuks

Ok, t'inquiète, c'était juste une simple question de curiosité :)

Changed 12 years ago by laurentj

Nouvelle version du patch, plus complète

comment:16 Changed 12 years ago by laurentj

Il y a encore quelques bugs dans un test unitaires, mais qui ne sont pas directement liés à priori avec les modifications des sélecteurs. Reste aussi à tester l'extension PHP.

comment:17 Changed 12 years ago by laurentj

Le patch entier a été intégré dans le trunk, ainsi que quelques corrections supplémentaires. Merci nuks pour ton patch !

Reste toutefois encore à tester l'extension PHP et un build de jelix avec ENABLE_OLD_ACTION_SELECTOR à false.

comment:18 Changed 12 years ago by bastnic

J'ignorais que vous alliez en arriver là quand j'avais posté ce bug.

C'est un changement massif d'API que vous avez mis en place là !!! Il va falloir tester ça en masse avant la version 1.0 finale.

Bravo pour tout ça !

/me retourne à sa cours de compilation

comment:19 Changed 12 years ago by nuks

Pas de problème ;) Un petit update et c'est ready ;)

comment:20 Changed 12 years ago by laurentj

Suite à une discussion sur IRC, on va améliorer la modification. Actuellement, on a deux modes avec l'option de config enableOldActionSelector :

  • =0 pas de support des anciens selecteurs. On doit utiliser les nouveaux selecteurs (avec le caractère ":" pour séparer controleur et methode), mais on peut utiliser toutes les variantes (genre possibilité d'indiquer seulement la méthode).
  • =1 on ne peut utiliser que les anciens selecteurs (avec le caractère "_" pour séparer controleur et methode)

C'est peut être pas assez souple. On pourrait, quand on met 1, autoriser aussi les nouveaux selecteurs, mais à la condition de toujours indiquer le controleur et la méthode pour les nouveaux sélecteurs. En d'autre terme, la présence ou pas d'un ":" dans le selecteur permettra de savoir dans le code si il s'agit d'un nouveau ou d'un ancien selecteur.

comment:21 Changed 12 years ago by laurentj

L'amélioration a été faite. On peut utiliser les nouveaux selecteurs quand enableOldActionSelector est à true, à condition d'indiquer le controleur et la méthode. Le fichier urls.xml et la section simple_urlengine_entrypoints dans la config doivent aussi contenir impérativement des selecteurs nouveau format.

comment:22 Changed 12 years ago by laurentj

  • Resolution set to fixed
  • Status changed from assigned to closed

Tout a été maintenant testé et corrigé. Le bug peut être fermé :-)

Note: See TracTickets for help on using tickets.