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

cargo shuttle init does not handle directory argument properly #665

Closed
jonaro00 opened this issue Mar 1, 2023 · 6 comments
Closed

cargo shuttle init does not handle directory argument properly #665

jonaro00 opened this issue Mar 1, 2023 · 6 comments
Assignees
Labels
T-Bug Something isn't working

Comments

@jonaro00
Copy link
Member

jonaro00 commented Mar 1, 2023

image

First invocation: The init command fails when the directory does not exist.

Second invocation: When directory exists, the command still asks for directory, even though it was provided. I manually typed it again.


Env: WSL 2, Ubuntu 22.04
Shuttle: 0.11.0
Rust and cargo: 1.66.1

@brokad brokad added the T-Bug Something isn't working label Mar 2, 2023
@h-dav
Copy link

h-dav commented Mar 2, 2023

I’ll sort this

@d2weber
Copy link
Contributor

d2weber commented Mar 8, 2023

Also, I noticed that, when I create a new project, the name in Cargo.toml seems to be wrong:

/tmp/pwd$ cargo shuttle init --axum
How do you want to name your project? It will be hosted at ${project_name}.shuttleapp.rs.
✔ Project name · myname

Where should we create this project?
✔ Directory · .

     Created library (shuttle) package

✔ Do you want to create the project environment on Shuttle? · no

/tmp/pwd$ rg name Cargo.toml 
2:name = "pwd"

Should I create a new issue?

@oddgrd
Copy link
Contributor

oddgrd commented Mar 17, 2023

As of #706, which will be released on monday, the directory argument will now be properly handled. It will not error, it will create the dir and present the path to it in the where would you like to create this project. I'll not close this issue yet, since we might want to improve it further. Do we want to prompt the user for a path at all if they gave one in the command? Also, should we make the command require --path <path> so it's more clear what the command does?

Regarding the naming @d2weber, yes, I think an issue for that is good. Did it really name your project "pwd"? 😂

@jonaro00
Copy link
Member Author

Great!

Do we want to prompt the user for a path at all if they gave one in the command?

This defeats the purpose of having the command line argument...? So I think no.

Also, should we make the command require --path <path> so it's more clear what the command does?

I also think this is not necessary, since basically all "init" commands work this way.

Other comments

I just remembered: The prompt for project name could use a reminder to (Choose a unique name).

The crate name that gets put into Cargo.toml might be incorrect: When doing init with project name hello-world and folder name hello and choosing yes for creating env, the creation request fails when trying to create a project hello-world (Is my guess; By the way, the error message should include the name that already existed, to be extra clear), but the crate is named hello after the folder name. This means that if attempting a new project new (without Shuttle.toml name), it attempts to use hello.

TLDR; The crate shall be named the same as the project OR a Shuttle.toml should be generated with proper name OR/AND the init command shall prompt the user for a new project name if the name already existed.

@oddgrd
Copy link
Contributor

oddgrd commented Mar 20, 2023

This is great, thanks @jonaro00! So to summarize the name fixes:

  • Generating a shuttle.toml with the project name
  • Suggesting to the user to pick a unique name
  • If the name is taken, show it in the error message when creating a problem (this may be made redundant in the init command if we don't error on name taken, but can be useful for other calls to project new)
  • If the name is taken, don't error, rather prompt to enter a new name and try to create a project again
  • And for the path issue, I agree with you, we can remove the directory prompt if they entered a path to the init command

Would you mind opening a new issue for this? And of course, if you'd like to work on this yourself we'd be happy to answer any questions you may have 🙂

@oddgrd
Copy link
Contributor

oddgrd commented Mar 22, 2023

On second thought, we should review #705 before opening another issue for init.

I'm also closing this issue since the path issue has been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants