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 +capitalize+ option to Inflector.humanize #12789

Merged
merged 1 commit into from Nov 6, 2013

Conversation

claudiob
Copy link
Member

@claudiob claudiob commented Nov 6, 2013

So strings can be humanized without being capitalized:

'employee_salary'.humanize                    # => "Employee salary"
'employee_salary'.humanize(capitalize: false) # => "employee salary"

@claudiob
Copy link
Member Author

claudiob commented Nov 6, 2013

@@ -190,13 +190,18 @@ def classify
ActiveSupport::Inflector.classify(self)
end

# Capitalizes the first word, turns underscores into spaces, and strips '_id'.
# Turns underscores into spaces, and strips a trailing '_id' if present.
Copy link
Member

Choose a reason for hiding this comment

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

The default behaviour should probably be mentioned here, maybe:

"Turns underscores into spaces, and strips a trailing '_id' if present. Also capitalizes the first word by default."

then later...

"The capitalization behaviour can be changed by ..."

Probably not the best possible way to say this, but my point is that it seems easier to read if we point out the default upfront (since that's how most people will use this) and point out what can be changed (+how) later.

@fxn
Copy link
Member

fxn commented Nov 6, 2013

Also, the patch needs to update the AS guide.

@claudiob
Copy link
Member Author

claudiob commented Nov 6, 2013

@fxn Thanks for the feedback… will be back soon with a new commit

So strings can be humanized without being capitalized:

    'employee_salary'.humanize                    # => "Employee salary"
    'employee_salary'.humanize(capitalize: false) # => "employee salary"
@claudiob
Copy link
Member Author

claudiob commented Nov 6, 2013

@fxn I think I've applied all your suggested changes… tell me if I missed anything! 🍭

@fxn
Copy link
Member

fxn commented Nov 6, 2013

Looking good, thanks!

fxn added a commit that referenced this pull request Nov 6, 2013
Add +capitalize+ option to Inflector.humanize
@fxn fxn merged commit f8e5022 into rails:master Nov 6, 2013
@claudiob claudiob deleted the humanize-without-capitalizing branch November 6, 2013 21:35
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.

None yet

4 participants