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

fix: disallow dashes in repo name #690

Closed
wants to merge 1 commit into from

Conversation

cfcosta
Copy link
Contributor

@cfcosta cfcosta commented Jul 28, 2023

When creating a new repo, it is currently allowed to create with a dash in the name, which immediately fails aiken check:

Screenshot 2023-07-28 at 2 19 25 PM

This fixes it.

@cfcosta cfcosta requested a review from rvcas July 28, 2023 17:35
@KtorZ
Copy link
Member

KtorZ commented Jul 28, 2023

I think the better fix would be to actually allow hyphens in module names 🧐

There's some stylistic debate with this probably and I am sure @rvcas has a strong opinion on it.

But I'd find this to be a quite saddening user experience to realize only after having created a repository that the name is deemed invalid by the compiler. Especially on something as common as an hyphen.

@rvcas
Copy link
Member

rvcas commented Jul 28, 2023

@KtorZ I told him to block it, we do not support kebab case as var names and will likely never support that and we'll probably also block camcelCase names. The alternative would be to catch this and then replace - with _ for them which can arguably be more confusing than presenting an error. but otherwise I would just block it.

That said I'd be pretty in favor of replacing - with _, seems chill and then you can name projects more flexibly. The only caveat is that the import will require an underscore. This is how it works in rust as well. For example if you name a crate aiken-lang it's available as aiken_lang.

@KtorZ
Copy link
Member

KtorZ commented Jul 28, 2023

I don't think replacing will work as it opens the gate to spoofing attacks now. Someone can register a project with underscore in the name and try to hijack an established one.

Given that repository names will never serve as variable identifiers, I think it's fine here to have the parser accept hyphens while still forbidding them in module and variable names

@rvcas
Copy link
Member

rvcas commented Jul 28, 2023

@KtorZ register a project? You mean in some registry? I believe crates.io handles it fine.

@rvcas rvcas closed this in 437a95b Aug 31, 2023
@rvcas rvcas deleted the cfcosta/disallow-dashes-in-repo-name branch August 31, 2023 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ❌ Canceled
Development

Successfully merging this pull request may close these issues.

3 participants