Review Request 117407: Add a project name validation in the project creation dialog

[Sven Brauch]

> On April 6, 2014, 11:55 p.m., Aleix Pol Gonzalez wrote:
> > plugins/appwizard/projectselectionpage.cpp, line 195
> > <https://git.reviewboard.kde.org/r/117407/diff/2/?file=263225#file263225line195>
> >
> >     How did you come up with this restriction?
> >     
> >     Why people can't put accents to the project name? Or even dashes?
> >     
> >     Definitely a no-go.
> 
> Simon Wächter wrote:
>     The restriction was added to avoid a project generation with invalid class names (C++ and afaik Python). Create a new project from a template and use some strange project name, it will result in a *broken* project.
>     
>     Maybe it's a better idea just to show a warning (--> Project name might lead to invalid class names) and let the user do the job ?
> 
> Sven Brauch wrote:
>     @Aleix: Try creating a project with such a name right now. It will not work for almost all the templates. So either someone has to fix all the templates, or we should restrict the possible names to those which actually work. And until someone wants to do the former, the latter is certainly better than leaving all the people who type spaces into their project name with completely broken projects.
> 
> Sven Brauch wrote:
>     Ah, re. warning: no, that's bad in my opinion. Programs shouldn't act like "hey, if you do this I will break in the following way", they should either disallow it or handle it properly.
> 
> Sven Brauch wrote:
>     Hi Aleix, do you still stick with your opinion on this? I still think it's a good improvement until somebody wants to implement a better solution.
> 
> Aleix Pol Gonzalez wrote:
>     If it's a limitation by the template, then maybe it could be the template offering the regular expression?
> 
> Sven Brauch wrote:
>     Sounds good, but how would that work in practice? As far as I can see, not even all templates are provided by kdevelop ...?
> 
> Aleix Pol Gonzalez wrote:
>     We introduce the regular expression on the ones in KDE, we assume it's ".+" otherwise.
> 
> Sven Brauch wrote:
>     I guess we could do that, though I would assume this one instead of .+ otherwise -- if the template supports non-alphanumeric characters, it should advertise that, it should not be assumed (because that assumption turned out to be false in most cases, I think).
>     
>     Simon, would you be interested in doing that?
> 
> Simon Wächter wrote:
>     This solutions sounds much better & more future proof. I will ask Anne-Marie from the KAppTemplate project and if it's okay I'll add a project specific validation-regex to the kdevplatform/kapptemplate templates
> 
> Phil Schaf wrote:
>     I don't like that solution either.
>     
>     Sure, where project names are used as class names, it won't work in old languages (it will work fine in e.g. Python by the way)
>     
>     But when used as  strings or in comments, everything goes.
>     
>     So the most common case are templates where the use of the same variable is partly restricted and partly not.
>     
>     Also, I hate the idea of defaulting to something less powerful. That way, nothing will ever go forward.
>     
>     My idea: placeholders in templates which only accept simpler stuff should advertise that and use a normalized version of that variable.

In Python you can't have class names with dashes or spaces either.

I'm happy with any solution which leads to a fix for this mess -- the first suggestion was the result of a "least effort" decision. If you want to do something better, sure ;)


- Sven


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117407/#review55099
-----------------------------------------------------------


On April 6, 2014, 7:06 p.m., Simon Wächter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117407/
> -----------------------------------------------------------
> 
> (Updated April 6, 2014, 7:06 p.m.)
> 
> 
> Review request for KDevelop and Sven Brauch.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> This patch adds a regex validation for the project name in the project creation dialog. The project name has to start with a/A-z/Z and can contain a/A-z/Z, 0-9 and the _ character.
> 
> The idea of this patch is to make an invalid class name in a new project impossible, for example "42fooclass" or "_barclass".
> 
> 
> Diffs
> -----
> 
>   plugins/appwizard/projectselectionpage.cpp 6270869 
> 
> Diff: https://git.reviewboard.kde.org/r/117407/diff/
> 
> 
> Testing
> -------
> 
> Tested with several different project strings
> 
> 
> Thanks,
> 
> Simon Wächter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140418/c8d6e398/attachment-0001.html>