no periods in new contract names#192
Conversation
| where | ||
| P: AsRef<Path>, | ||
| { | ||
| if name.contains('.') { |
There was a problem hiding this comment.
Rather than checking for individual invalid characters, we should rather only allow valid characters e.g. alphanumeric, underscores.
|
@ascjones good call, made that change and removed these invalid character checks. |
| { | ||
| if name.contains('-') { | ||
| anyhow::bail!("Contract names cannot contain hyphens"); | ||
| if !name.replace('_', "").chars().all(char::is_alphanumeric) { |
There was a problem hiding this comment.
| if !name.replace('_', "").chars().all(char::is_alphanumeric) { | |
| if !name.chars().all(|c| c.is_alphanumeric() || c == '_') { |
I think something like this is more readable (note I haven't checked whether that compiles or not)
There was a problem hiding this comment.
Good idea, probably a tad more efficient too. Made the change and it does indeed compile
| { | ||
| if name.contains('-') { | ||
| anyhow::bail!("Contract names cannot contain hyphens"); | ||
| if !name.chars().all(|c| c.is_alphanumeric() || c == '_') { |
There was a problem hiding this comment.
this allows _ as identifier which is invalid. however, I think this isn't too bad?
There was a problem hiding this comment.
Well spotted. Is that even a valid crate name?
There was a problem hiding this comment.
I just checked and cargo does not immediately put a stop to having _ as crate name but my filesystem definitely does. xD
There was a problem hiding this comment.
Note sure we really need to guard against this. What is your opinion?
3 participants
to address issue #34