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

Add in prompt for itch-project name. #184

Merged
merged 3 commits into from
Aug 4, 2024

Conversation

Sapein
Copy link
Collaborator

@Sapein Sapein commented Jul 20, 2024

We now ask for the project name on itch.io if the user provides a username for itch.io. This is optional, and if left-blank we fallback to project-name.

I have to do this weirdly because I can't use template placeholders in cargo-generate.toml for defaults, as far as I could tell. We could create some hooks, but that is complicated and there are some issues we could run-into so I went this route instead so we wouldn't have to worry as much about it.

To elaborate on the issues with hooks, I'm specifically talking about init hooks. init can have access to project-name but it is not guaranteed. So we would have to babysit running order, which can be an issue. Additionally, I'm not completely certain on if they run after the prompt, before the prompt, or during the prompt. In which case we might still run into issues depending on how fast the user moves and all that.

If we run using pre hooks, then it runs after the prompt as far as I can tell..which means the default wouldn't be shown to the user anyway, which is the same as the current behavior. The only advantage of pre hooks would be that we can eliminate a branch in the release workflow.

We now ask for the project name on itch.io if the user provides a
username for itch.io. This is optional, and if left-blank we
fallback to project-name.

I have to do this weirdly because I can't use template
placeholders in cargo-generate.toml for defaults, as far as I
could tell. We *could* create some hooks, but that is complicated
and there are some issues we could run-into so I went this route
instead so we wouldn't have to worry as much about it.
@janhohenheim
Copy link
Member

Thanks! Will review after the jam.

Copy link
Collaborator

@benfrankel benfrankel left a comment

Choose a reason for hiding this comment

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

Wording nit. itch.io project name vs project-name may be confusing.

cargo-generate.toml Outdated Show resolved Hide resolved
Co-authored-by: Ben Frankel <[email protected]>
Copy link
Member

@janhohenheim janhohenheim left a comment

Choose a reason for hiding this comment

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

Allowing myself to commit @benfrankel's suggestion in order to get this merged :)

@janhohenheim janhohenheim merged commit e02e74b into TheBevyFlock:cargo-generate Aug 4, 2024
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.

3 participants