Skip to content

fix(new): improve error message when project name does not match regex - #3902

Closed
beeman wants to merge 3 commits into
angular:masterfrom
beeman:issue-3816
Closed

beeman wants to merge 3 commits into
angular:masterfrom
beeman:issue-3816

Conversation

@beeman

@beeman beeman commented Jan 8, 2017

Copy link
Copy Markdown
Contributor

This fixes issue #3816 by making the error message explain better what the restrictions are:

$ ng new alpha-bravo-2
Project name "alpha-bravo-2" is not valid. New project names must
start with a letter, and must contain only alphanumeric characters or dashes.
When adding a dash the segment after the dash must start with a letter too.

Open to any suggestions to improve the language.

@baruchvlz

baruchvlz commented Jan 8, 2017 •

Copy link
Copy Markdown
Contributor

It'll be nice to be able to point to the part of the name that is causing the error. For example:

ng new alpha-bravo-2
Project name "alpha-bravo-2" is not valid. New project names must
start with a letter, and must contain only alphanumeric characters or dashes.
When adding a dash the segment after the dash must start with a letter too.
Error in alpha-bravo-2
                     ^

@beeman

beeman commented Jan 8, 2017

Copy link
Copy Markdown
Contributor Author

@baruchvlz I agree that that would be a nice addition.

I've been looking for a way to determine at which position a regex fails, but I can't seem to find an easy way. Any suggestions?

@baruchvlz

baruchvlz commented Jan 9, 2017 •

Copy link
Copy Markdown
Contributor

@beeman I'm thinking maybe we can use RegExp.exec() to find the illegal character then use indexOf to set the position? But this may not be the best way to go at it.

Something like so:

var string = 'alpha-bravo-2';
string.indexOf(/[0-9]/.exec(string)); // 12

Obviously the RegExp will be more complex than this, but it's just an example. What do you think?

@beeman

beeman commented Jan 9, 2017

Copy link
Copy Markdown
Contributor Author

@baruchvlz I've been trying to get the index of the failing char but I'm not sure, it looks like RegExp.exep() does not return what we want.

Here is the playground I set up using the RegExp from the project: https://jsbin.com/yokerac/1/edit?js,console

@beeman

beeman commented Jan 9, 2017

Copy link
Copy Markdown
Contributor Author

@baruchvlz I've added an indicator that shows where the package name fails:

screen shot 2017-01-08 at 22 55 22

Thanks to @delasteve for helping me with the method that finds the position.

@filipesilva

filipesilva commented Jan 9, 2017 •

Copy link
Copy Markdown
Contributor

@beeman this is pretty useful!

@filipesilva filipesilva left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing missing would be a test. An acceptance test should be enough. See here for the current suite:
https://github.com/angular/angular-cli/blob/master/tests/acceptance/new.spec.ts

@beeman

beeman commented Jan 10, 2017

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing @filipesilva!

I've been looking at the current acceptance test, but the only seem to validate the RegExp, which did not change in this PR.

https://github.com/angular/angular-cli/blob/master/tests/acceptance/new.spec.ts#L58-L83

The only thing that's enhanced is the error message.

@filipesilva

Copy link
Copy Markdown
Contributor

@beeman a test making sure that the message is correct, and that it shows the error in the right place, is what I was thinking about.

@beeman
beeman deleted the issue-3816 branch February 2, 2017 02:59
MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 11, 2019
mostafaibr pushed a commit to mostafaibr/angular-cli that referenced this pull request May 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants