-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
iwahbe/7858/fix nodejs hypen imports #7993
Conversation
Diff for pulumi-azuread with merge commit 94e9aa9 |
Diff for pulumi-random with merge commit 94e9aa9 |
Diff for pulumi-random with merge commit 1974ac2 |
Diff for pulumi-azuread with merge commit 1974ac2 |
Diff for pulumi-kubernetes with merge commit 94e9aa9 |
Diff for pulumi-kubernetes with merge commit 1974ac2 |
Diff for pulumi-gcp with merge commit 94e9aa9 |
Diff for pulumi-aws with merge commit 94e9aa9 |
Diff for pulumi-gcp with merge commit 1974ac2 |
Diff for pulumi-aws with merge commit 1974ac2 |
Diff for pulumi-azure with merge commit 1974ac2 |
Diff for pulumi-azure with merge commit 94e9aa9 |
pkg/codegen/nodejs/gen.go
Outdated
// title Capitalizes the first rune in s. | ||
// | ||
// Note: this is expected to work on strings which are not valid identifiers. | ||
// Example: | ||
// "hello" => "Hello" | ||
// "hiAlice" => "HiAlice" | ||
// "hi.Bob" => "Hi.Bob" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
func pascal(s string) string { | ||
split := [][]rune{{}} | ||
runes := []rune(s) | ||
for _, r := range runes { | ||
if !isLegalIdentifierPart(r) && r != '.' { | ||
split = append(split, []rune{}) | ||
} else { | ||
split[len(split)-1] = append(split[len(split)-1], r) | ||
} | ||
} | ||
words := make([]string, len(split)) | ||
for i, v := range split { | ||
words[i] = title(string(v)) | ||
} | ||
return strings.Join(words, "") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there's already a camel
function could pascal
just be title(camel(s))
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I had that thought, but it doesn't work out. camel
does conversions like JSONObject
to jsonObject
. title(camel("JSONObject"))
would be JsonObject
and we want it to remain unchanged. camel
also has no impact on -
, which we want pascal
to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expanding on the comment for pascal
to include what you just said would be awesome!
Diff for pulumi-azuread with merge commit 29c605c |
Diff for pulumi-random with merge commit 29c605c |
Diff for pulumi-kubernetes with merge commit 29c605c |
Diff for pulumi-aws with merge commit 29c605c |
Diff for pulumi-gcp with merge commit 29c605c |
Diff for pulumi-azure with merge commit 29c605c |
Diff for pulumi-azuread with merge commit 5b4b883 |
Diff for pulumi-kubernetes with merge commit 5b4b883 |
Diff for pulumi-aws with merge commit 5b4b883 |
Diff for pulumi-gcp with merge commit 5b4b883 |
Diff for pulumi-azure with merge commit 5b4b883 |
Diff for pulumi-aws with merge commit 01b6895 |
Diff for pulumi-azuread with merge commit 01b6895 |
Diff for pulumi-random with merge commit 01b6895 |
Diff for pulumi-kubernetes with merge commit 01b6895 |
Diff for pulumi-gcp with merge commit 01b6895 |
Diff for pulumi-azure with merge commit 01b6895 |
Description
Encapsulate the creation of external package names (
pulumi%s
) and convert them to pascal case (correctly handling-
).Fixes #7858
Checklist