developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.
Opened 11 years ago
Closed 11 years ago
#1163 closed bug (fixed)
createapp -withcmdline fail in case of non alphanumeric characters in appname
| Reported by: | catsoup | Owned by: | |
|---|---|---|---|
| Priority: | highest | Milestone: | Jelix 1.1.7 |
| Component: | jelix-scripts | Version: | 1.1.6 |
| Severity: | normal | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Documentation needed: | no | |
| Hosting Provider: | Php version: |
Description
All is in the title. I made a patch, it applied successfully on jelix-trunk and jelix-1.1.x
Attachments (1)
Change History (8)
Changed 11 years ago by catsoup
comment:1 Changed 11 years ago by catsoup
- review set to review?
comment:2 Changed 11 years ago by catsoup
Update: added to my patch-queue http://bitbucket.org/catsoup/jelix-1.1.x-patches http://bitbucket.org/catsoup/jelix-trunk-patches patch name: jelix-scripts-createapp-withcmdline-fail-bugfix.patch
comment:3 Changed 11 years ago by laurentj
- Milestone set to Jelix 1.1.7
- review changed from review? to review-
- Version changed from trunk to 1.1.6
Because $GLOBALSAPPNAME? is used in many other place, your patch should contains other changes.
To avoid all of these changes, perhaps the best way is to check the given APPNAME before its use, so in jelix.php. For example, by adding a "else" statement on the statement
{{{ if ($APPNAME == && $command->applicationRequired) { } else if ($APPNAME != ) {
do something here.
} }}}
Two solutions:
- replace silently all bad characters as we do for the module name
- or check the name and throw an exception if it's bad.
The second solution is probably better. What do you think about it ?
comment:4 Changed 11 years ago by catsoup
Hello ! My patch corresponds to the first solution. I think it's more user friendly.
But, I don't know where it could break other things, concerning $GLOBALSAPPNAME? I looked a little, but can't find.
In the patch, the only change is the call of createentrypoint and createctrl with the normalized appname (which becomes the default module name) instead of original appname.
So as it is the same behaviour as for a no cmdline app, i don't see where there could have a break down somewhere ?
comment:5 Changed 11 years ago by catsoup
Sorry, I misread, my patch doesn't really corresponds to the first solution, it replace the default module name, not the appname.
About my opinion, I really don't know, because for both solutions, if I well understood, special characters will be disallowed in appname.
comment:6 Changed 11 years ago by laurentj
- review changed from review- to review+
Oh yes, sorry, I'm tired. your patch is good.
comment:7 Changed 11 years ago by laurentj
- Resolution set to fixed
- Status changed from new to closed
ok for trunk and 1.1.x