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 better error message for invalid crate names #7603

Merged
merged 5 commits into from
Nov 30, 2023

Conversation

Rustin170506
Copy link
Member

@Rustin170506 Rustin170506 commented Nov 24, 2023

Split from #7379

The summary:

category Cargo crates.io before change crates.io after change
Crate name 1. cannot start with a number
2. can only start with most letters or _
3. can only contain numbers, -, _, or most letters.
1. must start with alphabetic
2. can only contain numbers, letters, -, _
1. must start with alphabetic
2. can only contain numbers, letters, -, _ (Nothing changes)

Copy link
Member Author

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

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

🔢 Self-check

@Rustin170506 Rustin170506 requested a review from a team November 24, 2023 15:40
@Rustin170506 Rustin170506 marked this pull request as ready for review November 24, 2023 15:40
@Rustin170506 Rustin170506 changed the title Add better error message for invalid crate name Add better error message for invalid crate names Nov 24, 2023
"detail": "\"🦀\" is an invalid dependency name (dependency names must start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters)"
"detail": "invalid character `🦀` in crate name: `🦀`, the first character must be an ASCII character"
Copy link
Member

Choose a reason for hiding this comment

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

previously the error message was warning about a dependency name, but now it is only saying "crate name". maybe we need another error wrapper to give more context in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, nice catch. I will update it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I submitted ccbcaee

I don't know if this is a good way to do it. Maybe it just introduces more confusion.

Do you have any suggestions to improve it? Or maybe we can keep it as the crate name. Because it literally is a crate name.

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-backend ⚙️ labels Nov 29, 2023
@Rustin170506 Rustin170506 requested a review from Turbo87 November 29, 2023 12:24
@Turbo87 Turbo87 merged commit 0fdf754 into rust-lang:main Nov 30, 2023
6 checks passed
@Rustin170506 Rustin170506 deleted the rustin-patch-crate-name branch November 30, 2023 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants