Skip to content

Conversation

santry
Copy link

@santry santry commented Apr 13, 2017

This PR adds the ability to pass a list of acronyms to be capitalized when titleizing a string. It also includes the ability to pass a custom list of small words, which are added to the built-in list.

When ActiveSupport is loaded, titleize now will also respect the acronyms configured in ActiveSupport::Inflector.inflections.acronyms.


it "should call Titleize#titleize" do
Titleize.should_receive(:titleize).with("title")
Titleize.should_receive(:titleize).with("title", {})

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.

it "should properly capitalize acronyms configured in Inflector.inflections.acronyms" do
inflections = double(acronyms: ['SEC'])
ActiveSupport::Inflector.stub!(:inflections).and_return(inflections)
"SMITH TO HEAD SEC".titleize.should == 'Smith to Head SEC'

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.

"title".titleize(:humanize => false)
end

it "should properly capitalize acronyms configured in Inflector.inflections.acronyms" do

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.
Line is too long. [92/80]

end

it "should not capitalize words that have a lowercase first letter" do
it "should not capitalize words that have a lowercase first letter" do

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.

end
end

it "should not capitalize words with dots" do

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.

end
end

it "should not capitalize custom small words specified in opts[:small_words]" do

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.
Line is too long. [84/80]

#stub
def underscore(string) string; end
def humanize(string) string; end
def inflections

Choose a reason for hiding this comment

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

Use empty lines between method definitions.

# for ActiveSupport::Inflector
opts[:acronyms] ||= ActiveSupport::Inflector.inflections.acronyms

passthru_opts = opts.select { |k, _| [:acronyms, :small_words].include?(k) }

Choose a reason for hiding this comment

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

Line is too long. [82/80]

# Inflector.underscore and Inflector.humanize to convert
# underscored_names and CamelCaseNames to a more human form. However, you can change
# this behavior by passing :humanize => false or :underscore => false as options.
# this behavior by passing :humanize => false or :underscore => false as options.

Choose a reason for hiding this comment

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

Line is too long. [85/80]


def titleize!
replace(titleize)
def titleize!(opts={})

Choose a reason for hiding this comment

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

Surrounding space missing in default value assignment.

@santry santry force-pushed the custom-small-words-and-acronyms branch from 8503b98 to 1847ca8 Compare April 13, 2017 17:27
# capitalized. To override the set of acronyms used, pass in
# opts[:acronyms]
#
# titleize("SMITH TO HEAD SEC", acronyms: ['SEC']) # => "Smith to Head SEC"

Choose a reason for hiding this comment

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

Line is too long. [81/80]

@santry santry force-pushed the custom-small-words-and-acronyms branch from 1847ca8 to 93efd76 Compare April 13, 2017 19:48
# for ActiveSupport::Inflector
opts[:acronyms] ||= ActiveSupport::Inflector.inflections.acronyms.values

passthru_opts = opts.select { |k, _| [:acronyms, :small_words].include?(k) }

Choose a reason for hiding this comment

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

Line is too long. [82/80]

@granth
Copy link
Owner

granth commented Apr 25, 2017

Thanks! This looks good, and I think I'll merge it with some minor changes.

I'll get Hound to stand down on some of its issues – single quotes for sure.

It also takes care of the languishing #11.

@santry
Copy link
Author

santry commented Apr 25, 2017

Sounds good. I left hound's howls alone since what I added seemed to (mostly) match what I saw in the existing code. If you need me to make changes I'm happy to, or, of course, you can hack it as you wish!

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