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 10 years ago

Closed 10 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)

jelix-scripts-createapp-withcmdline-fail-bugfix.diff (1.0 KB) - added by catsoup 10 years ago.
ok for trunk and 1.1.x

Download all attachments as: .zip

Change History (8)

Changed 10 years ago by catsoup

ok for trunk and 1.1.x

comment:1 Changed 10 years ago by catsoup

  • review set to review?

comment:2 Changed 10 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 10 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 10 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 10 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 10 years ago by laurentj

  • review changed from review- to review+

Oh yes, sorry, I'm tired. your patch is good.

comment:7 Changed 10 years ago by laurentj

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.