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 enum capitalization #2630

Merged
merged 2 commits into from
May 11, 2023

Conversation

dukhyungkim
Copy link
Contributor

First of all, sorry for my bad english.

In my case, I want to use IDLE and it should be Idle. But the generated output shows me IDLe.
And I found similar issue #2469. So I try to satisfy both.

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@coveralls
Copy link

coveralls commented May 9, 2023

Coverage Status

Coverage: 78.797% (-0.01%) from 78.807% when pulling 401589b on dukhyungkim:enum_capitalization into e62a027 on 99designs:master.

if commonInitialisms[strings.ToUpper(word)] {
upperWord := strings.ToUpper(word)
if commonInitialisms[upperWord] {
switch upperWord {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
switch upperWord {
// If the uppercase word (string(runes[w:i]) is "ID" or "IP"
// AND
// the word is the first two characters of the str
// AND
// that is not the end of the word
// AND
// the length of the string is greater than 3
// AND
// the third rune is an uppercase one
// THEN
// do NOT count this as an initialism.
switch upperWord {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion!
I applied it and pushed.

@StevenACoffman
Copy link
Collaborator

StevenACoffman commented May 11, 2023

Thanks! It took me a little while to understand what this code was doing. I suggested adding a comment (which will probably need to be indented differently), because in the future it may help someone to make this a more general fix than just for ID and IP.

@StevenACoffman StevenACoffman merged commit 33fdd1b into 99designs:master May 11, 2023
@StevenACoffman
Copy link
Collaborator

Thanks for making this improvement! Keep them coming!

@dukhyungkim
Copy link
Contributor Author

dukhyungkim commented May 11, 2023

Thanks! It took me a little while to understand what this code was doing. I suggested adding a comment (which will probably need to be indented differently), because in the future it may help someone to make this a more general fix than just for ID and IP.

Agree. I worried while I was writing this PR because it looks too tricky.

Thanks for making this improvement! Keep them coming!

Thank you for accepting. I look forward to meet again on PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants