Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 3 commits into from

Conversation

beeman
Copy link
Contributor

@beeman beeman commented Jan 8, 2017

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
Copy link
Contributor

baruchvlz commented Jan 8, 2017

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
Copy link
Contributor Author

beeman commented Jan 8, 2017

@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
Copy link
Contributor

baruchvlz commented Jan 9, 2017

@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
Copy link
Contributor Author

beeman commented Jan 9, 2017

@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
Copy link
Contributor Author

beeman commented Jan 9, 2017

@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
Copy link
Contributor

filipesilva commented Jan 9, 2017

@beeman this is pretty useful!

Copy link
Contributor

@filipesilva filipesilva left a comment

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
Copy link
Contributor Author

beeman commented Jan 10, 2017

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
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

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
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