-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Improve error message when user specifies an invalid template #2992
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
base: main
Are you sure you want to change the base?
Conversation
throwInvalidTemplateNameUsedError :: Command a | ||
throwInvalidTemplateNameUsedError = | ||
throwProjectCreationError $ | ||
"Are you sure that the template exists?" | ||
<> " 🤔 Check the list of templates here: https://github.com/wasp-lang/starters" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this out of common into a local function:
- It's very specific and only used in a single place.
- The function was missing information to output a meaningful error message (chosen template name, available templates). Both things are available in the caller.
findTemplateOrThrow templateName = case findTemplateByString availableTemplates templateName of | ||
Just x -> return x | ||
Nothing -> throwProjectCreationError $ makeInvalidTemplateNameError templateName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't use maybe
on purpose, I find this more readable.
I can use maybe
if anyone objects :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small naming suggestion
findTemplateByString availableTemplates templateName | ||
& maybe throwInvalidTemplateNameUsedError return | ||
findTemplateOrThrow templateName = case findTemplateByString availableTemplates templateName of | ||
Just x -> return x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe swap x
with starterTemplate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made some extra changes. We now always list all the templates explicitly but never point to the repository with the templates.
I think this is an improvement because users know what they have to input without leaving the terminal.
The downside is that it's not as easy to explore the templates before creating a project with one, but I don't think this is important (especially because the old link pointed only to the starters
repo, without mentioning OpenSaas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, looks good! I trust you tested the changes, so I didn't.
Fixes #2960