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

Error at new time if name is not usable #2056

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

itowlson
Copy link
Collaborator

@itowlson itowlson commented Nov 5, 2023

Fixes #2052.

# Error cases from original issue
ivan@hecate:~/testing$ spin new -t http-rust 2023-11-spin
error: Invalid value "2023-11-spin" for '<NAME>': Each segment of the name must start with a letter. '2023', '11' do not start with a letter

ivan@hecate:~/testing$ spin new -t http-rust spin-2023-11
error: Invalid value "spin-2023-11" for '<NAME>': Each segment of the name must start with a letter. '2023', '11' do not start with a letter

ivan@hecate:~/testing$ spin new -t http-rust spin_2023_11
error: Invalid value "spin_2023_11" for '<NAME>': Each segment of the name must start with a letter. '2023', '11' do not start with a letter

# Singular form
ivan@hecate:~/testing$ spin new -t http-rust spin-2
error: Invalid value "spin-2" for '<NAME>': Each segment of the name must start with a letter. '2' does not start with a letter

# This is caught but the error message is not ideal: is this required to fix?
ivan@hecate:~/testing$ spin new -t http-rust _foo
error: Invalid value "_foo" for '<NAME>': Each segment of the name must start with a letter. '' does not start with a letter

# Accepted
ivan@hecate:~/testing$ spin new -t http-rust spin_y2023_m11

(add uses the same logic and gets the same errors.)

Long term it would be good to centralise this logic, but the new/add name rules are slightly more forgiving than the manifest parser (because the templates kebab-case the things we care about kebab-casing), so I didn't tackle that here.

@itowlson itowlson linked an issue Nov 5, 2023 that may be closed by this pull request
3 tasks
Ok(name.to_owned())
} else {
Err("Name must start with a letter".to_owned())
let splits = name.split(|c| !char::is_alphanumeric(c));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can the separator legally be any non-alphanumeric character? Is "foo😴a" a legal name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, it works, so I guess it's de facto legal! The name as written is only used for the app name, which Spin doesn't care about. Component names get kebab-cased during processing, which turns that into foo-a which is a legal kebab-id. And the generated directory gets snake-cased to foo_a which is a legal directory name.

Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@itowlson itowlson requested a review from rylev November 6, 2023 22:57
@itowlson itowlson merged commit 7b04f2f into spinframework:main Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spin new: trouble with name using number Validate component name during spin new, spin add, and spin build
3 participants