Titlecase replaces "::" with "/" #3662

Closed
Mange opened this Issue Nov 17, 2011 · 19 comments

Comments

Projects
None yet
8 participants
@Mange

Mange commented Nov 17, 2011

String#titlecase is replacing all instances of "::" with "/" (since it's aliased from #titleize). I understand this is to format Ruby class names, but could somebody tell me the rationale behind this?

Shouldn't titlecase be similar to upcase and downcase? If somebody wants to format a class name through humanize first, shouldn't they just do that?

"foo :: bar".titlecase # => "Foo / Bar"

# Expected "Foo :: Bar"

At least, if this is intentional for some reason, the documentation should say that "Titlecase is made for formatting class names, and nothing else".

[EDIT: Changed the title and body of this issue to be more in line with the discussion. It talked about titleize previously since they are aliases with each other, but that muddied the discussion. The main point here is that the alias should not be there.]

@frodsan

This comment has been minimized.

Show comment Hide comment
@frodsan

frodsan Apr 29, 2012

Contributor

@Mange titleize: "Capitalizes all the words and replaces some characters in the string to create a nicer looking title. titleize is meant for creating pretty output. It is not used in the Rails internals.". Is this still an issue?

Contributor

frodsan commented Apr 29, 2012

@Mange titleize: "Capitalizes all the words and replaces some characters in the string to create a nicer looking title. titleize is meant for creating pretty output. It is not used in the Rails internals.". Is this still an issue?

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Apr 29, 2012

Owner

I don't think so. Closing it now. If you can point why this is still an issue I can reopen it.

Thanks for reporting.

Owner

rafaelfranca commented Apr 29, 2012

I don't think so. Closing it now. If you can point why this is still an issue I can reopen it.

Thanks for reporting.

@Mange

This comment has been minimized.

Show comment Hide comment
@Mange

Mange Apr 30, 2012

So how does "/" make it a nicer looking title? It's not nicer at all, unless you're formatting class names (and I still don't agree with that).

I'll try to explain where I'm coming from again. Excuse me if it sounds rantish, I just want to get it all in there in one comment. :-)

If the method is named titlecase it should just change case. I think you should either remove the alias or make the methods separate. It's on String, next to String#downcase and String#upcase.

Capitalizes all the words and replaces some characters

That sounds like format_as_title (or more succinct: titleize)

Capitalizes all letters

That sounds like upcase

Downcases all letters

That sounds like downcase

Capitalizes all the words

That sounds like titlecase

As long as ActiveSupport is duck punching String, it should add stuff that is a little less Rails-oriented. Replacing some random characters is just magic.

Do you still think titlecase should work this way the day someone decides to remove "!" as well? Question marks? Changing "->" into arrows? It's just not something any ruby developer expects a method like titlecase to do. titleize, sure, since that implies some unspecified tranformation, but titlecase only imples case change.

Here's a simple rule-of-thumb:

If Ruby 2.0 implemented String#titlecase, would it work like this? No, it would not.

Mange commented Apr 30, 2012

So how does "/" make it a nicer looking title? It's not nicer at all, unless you're formatting class names (and I still don't agree with that).

I'll try to explain where I'm coming from again. Excuse me if it sounds rantish, I just want to get it all in there in one comment. :-)

If the method is named titlecase it should just change case. I think you should either remove the alias or make the methods separate. It's on String, next to String#downcase and String#upcase.

Capitalizes all the words and replaces some characters

That sounds like format_as_title (or more succinct: titleize)

Capitalizes all letters

That sounds like upcase

Downcases all letters

That sounds like downcase

Capitalizes all the words

That sounds like titlecase

As long as ActiveSupport is duck punching String, it should add stuff that is a little less Rails-oriented. Replacing some random characters is just magic.

Do you still think titlecase should work this way the day someone decides to remove "!" as well? Question marks? Changing "->" into arrows? It's just not something any ruby developer expects a method like titlecase to do. titleize, sure, since that implies some unspecified tranformation, but titlecase only imples case change.

Here's a simple rule-of-thumb:

If Ruby 2.0 implemented String#titlecase, would it work like this? No, it would not.

@rafaelfranca rafaelfranca reopened this Apr 30, 2012

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Apr 30, 2012

Owner

@Mange good point. I'm reopening it and calling @fxn for discussion. Thank.

Owner

rafaelfranca commented Apr 30, 2012

@Mange good point. I'm reopening it and calling @fxn for discussion. Thank.

@fxn

This comment has been minimized.

Show comment Hide comment
@fxn

fxn Apr 30, 2012

Owner

ACK, I have a queue of stuff to look into but I will eventually get to this.

Owner

fxn commented Apr 30, 2012

ACK, I have a queue of stuff to look into but I will eventually get to this.

@ghost ghost assigned fxn Apr 30, 2012

@Mange

This comment has been minimized.

Show comment Hide comment
@Mange

Mange Apr 30, 2012

Thanks for taking the time, guys! 👍

Mange commented Apr 30, 2012

Thanks for taking the time, guys! 👍

@markmcspadden

This comment has been minimized.

Show comment Hide comment
@markmcspadden

markmcspadden May 8, 2012

Contributor

Spent some time digging into this today and after playing around some, I have to say I'm not super on board with a change here. (Just one guy's opinion.)

The overall theme I think I keep coming back to is trying to keep expected results across inflections. I'd rather know that '::' gets changed to '/' throughout the inflections than have to keep up with which one I'm using and what special cases that one does or does not handle. (Does it change camelcase? underscores? dashes?) That said...it doesn't look like #humanize is especially consistant in those respects...but I'd vote for bringing it closer to #titleize and #underscore than the other way around.

A few things to consider:

  1. https://github.com/rails/rails/blob/4d0e6db9ec7537401d05f523ba8b29955f84f846/activesupport/test/inflector_test.rb#L101-144 - Even though these cases are focused the acronyms, they give a good idea of how these different inflections are expected to play nice together. (That said, that table is misleading if you ask me...especially the 'humanize' column.)

  2. Even one of the documented examples shows a changed character for pretty sake: 'x-men: the last stand'.titleize # => "X Men: The Last Stand" - changes the dash to a space. (Ok...it's less a change for pretty sake and more due to relying on #underscore.)

I guess I'm more onboard with documenting the characters that #titleize does change (::,-,_) and leaving as is.

I did go ahead and push a branch with some failing "Foo::Bar" tests in case anyone else wants a go at it. https://github.com/markmcspadden/rails/tree/issue_3662

Contributor

markmcspadden commented May 8, 2012

Spent some time digging into this today and after playing around some, I have to say I'm not super on board with a change here. (Just one guy's opinion.)

The overall theme I think I keep coming back to is trying to keep expected results across inflections. I'd rather know that '::' gets changed to '/' throughout the inflections than have to keep up with which one I'm using and what special cases that one does or does not handle. (Does it change camelcase? underscores? dashes?) That said...it doesn't look like #humanize is especially consistant in those respects...but I'd vote for bringing it closer to #titleize and #underscore than the other way around.

A few things to consider:

  1. https://github.com/rails/rails/blob/4d0e6db9ec7537401d05f523ba8b29955f84f846/activesupport/test/inflector_test.rb#L101-144 - Even though these cases are focused the acronyms, they give a good idea of how these different inflections are expected to play nice together. (That said, that table is misleading if you ask me...especially the 'humanize' column.)

  2. Even one of the documented examples shows a changed character for pretty sake: 'x-men: the last stand'.titleize # => "X Men: The Last Stand" - changes the dash to a space. (Ok...it's less a change for pretty sake and more due to relying on #underscore.)

I guess I'm more onboard with documenting the characters that #titleize does change (::,-,_) and leaving as is.

I did go ahead and push a branch with some failing "Foo::Bar" tests in case anyone else wants a go at it. https://github.com/markmcspadden/rails/tree/issue_3662

@Mange

This comment has been minimized.

Show comment Hide comment
@Mange

Mange May 9, 2012

Interesting test cases under 1. I still don't think these make any sense, though:

    #  camelize             underscore            humanize              titleize
    [
      ["Nokogiri::HTML",    "nokogiri/html",      "Nokogiri/HTML",      "Nokogiri/HTML"],
      ["HTTP::Get",         "http/get",           "HTTP/get",           "HTTP/Get"],
    ]

I feel that these cases aren't saying what the values should be, but rather document what the value becomes due to how it's implemented. I think this would make more sense:

    #  camelize             underscore            humanize              titleize
    [
      ["Nokogiri::HTML",    "nokogiri/html",      "Nokogiri::HTML",     "Nokogiri::HTML"],
      ["HTTP::Get",         "http/get",           "HTTP::get",          "HTTP::Get"],
    ]

The example you mention under 2 is the same way; just telling people that "this does not work like you expect".

This is of course subjective per definition, and also besides the point. titleize can work any way, IMO. :-)

I'm more concerned about the aliased titlecase. case. It does not conform to the other String#<whatever>case methods. I should change the name of this issue.

Mange commented May 9, 2012

Interesting test cases under 1. I still don't think these make any sense, though:

    #  camelize             underscore            humanize              titleize
    [
      ["Nokogiri::HTML",    "nokogiri/html",      "Nokogiri/HTML",      "Nokogiri/HTML"],
      ["HTTP::Get",         "http/get",           "HTTP/get",           "HTTP/Get"],
    ]

I feel that these cases aren't saying what the values should be, but rather document what the value becomes due to how it's implemented. I think this would make more sense:

    #  camelize             underscore            humanize              titleize
    [
      ["Nokogiri::HTML",    "nokogiri/html",      "Nokogiri::HTML",     "Nokogiri::HTML"],
      ["HTTP::Get",         "http/get",           "HTTP::get",          "HTTP::Get"],
    ]

The example you mention under 2 is the same way; just telling people that "this does not work like you expect".

This is of course subjective per definition, and also besides the point. titleize can work any way, IMO. :-)

I'm more concerned about the aliased titlecase. case. It does not conform to the other String#<whatever>case methods. I should change the name of this issue.

@Mange

This comment has been minimized.

Show comment Hide comment
@Mange

Mange May 9, 2012

There, I changed the name and the description of this issue. I propose that we leave titleize just like how it is today. No need to discuss what constitutes a nicer formatted title. :-)

Let's just discuss this problem:

"Foo :: BAR :: baz".upcase # => "FOO :: BAR :: BAZ"
"Foo :: BAR :: baz".downcase # => "foo :: bar :: baz"
"Foo :: BAR :: baz".titlecase # => "Foo / Bar / Baz" - what!?

It's not a nice experience at all, and tracking down this method to ActiveSupport can be tricky – especially when ActiveSupport is included as a dependency to a gem and you don't expect it at all.

Mange commented May 9, 2012

There, I changed the name and the description of this issue. I propose that we leave titleize just like how it is today. No need to discuss what constitutes a nicer formatted title. :-)

Let's just discuss this problem:

"Foo :: BAR :: baz".upcase # => "FOO :: BAR :: BAZ"
"Foo :: BAR :: baz".downcase # => "foo :: bar :: baz"
"Foo :: BAR :: baz".titlecase # => "Foo / Bar / Baz" - what!?

It's not a nice experience at all, and tracking down this method to ActiveSupport can be tricky – especially when ActiveSupport is included as a dependency to a gem and you don't expect it at all.

@steveklabnik

This comment has been minimized.

Show comment Hide comment
@steveklabnik

steveklabnik May 16, 2012

Member

Think of namespaces and breadcrumbs and page titles. The slash makes perfect sense to me.

Maybe I'm just strange.

Member

steveklabnik commented May 16, 2012

Think of namespaces and breadcrumbs and page titles. The slash makes perfect sense to me.

Maybe I'm just strange.

@Mange

This comment has been minimized.

Show comment Hide comment
@Mange

Mange May 16, 2012

Then why would you format the titles with double colons before calling titlecase? It's helpful if you're doing it wrong from the start.

Mange commented May 16, 2012

Then why would you format the titles with double colons before calling titlecase? It's helpful if you're doing it wrong from the start.

@steveklabnik

This comment has been minimized.

Show comment Hide comment
@steveklabnik

steveklabnik May 16, 2012

Member

Converting namespaced classes to a string. shrug.

Member

steveklabnik commented May 16, 2012

Converting namespaced classes to a string. shrug.

@Mange

This comment has been minimized.

Show comment Hide comment
@Mange

Mange May 16, 2012

Indeed. It's a method to format class names, not to change the case of a string (which is the point of this ticket).

Again, we can keep the current semantics for #titleize in case some people format class names for titles[1]. :-)
I discovered this when I was developing a non-rails app that had a dependency on a gem that has a dependency of ActiveSupport. It changed String under my feet to something that looked like it was working until I got data with "::" or "-" in it.

[1] I think that's pretty short-sighted. There's I18n for that, and class names should be able to be refactored without affecting user-facing views. Not going to argue about it, though. I see the point in doing it.

Mange commented May 16, 2012

Indeed. It's a method to format class names, not to change the case of a string (which is the point of this ticket).

Again, we can keep the current semantics for #titleize in case some people format class names for titles[1]. :-)
I discovered this when I was developing a non-rails app that had a dependency on a gem that has a dependency of ActiveSupport. It changed String under my feet to something that looked like it was working until I got data with "::" or "-" in it.

[1] I think that's pretty short-sighted. There's I18n for that, and class names should be able to be refactored without affecting user-facing views. Not going to argue about it, though. I see the point in doing it.

@fxn

This comment has been minimized.

Show comment Hide comment
@fxn

fxn May 28, 2012

Owner

My hypothesis is that it is just a consequence of the way it is implemented.

This inflection is supposed to yield pretty output, but its implementation is based on underscore and humanize. As you know underscore is meant to turn class names into file paths, and humanize is thought to put some makeup to attribute names. There's no way you can get a titlecase helper without weird edge cases from that combination.

I'd be willing to rewrite this helper because as a user I'd expect "x-men".titleize not to touch the hyphen, and "ActiveRecord::Base code walkthrough".titleize not to touch the class name. The normal use cases (eg, "top gun") would yield the same result, and I find hard to believe that someone relies on disappearing hyphens and strange replacements like those, because I find hard to find a use case where that output is the one you seek.

Plus, we are heading a major release and can afford some edge-cases backwards incompatibility.

So, 👍 from my side to rethink the test suite and to rewrite the helper.

@jeremy, what do you think?

Owner

fxn commented May 28, 2012

My hypothesis is that it is just a consequence of the way it is implemented.

This inflection is supposed to yield pretty output, but its implementation is based on underscore and humanize. As you know underscore is meant to turn class names into file paths, and humanize is thought to put some makeup to attribute names. There's no way you can get a titlecase helper without weird edge cases from that combination.

I'd be willing to rewrite this helper because as a user I'd expect "x-men".titleize not to touch the hyphen, and "ActiveRecord::Base code walkthrough".titleize not to touch the class name. The normal use cases (eg, "top gun") would yield the same result, and I find hard to believe that someone relies on disappearing hyphens and strange replacements like those, because I find hard to find a use case where that output is the one you seek.

Plus, we are heading a major release and can afford some edge-cases backwards incompatibility.

So, 👍 from my side to rethink the test suite and to rewrite the helper.

@jeremy, what do you think?

@schneems

This comment has been minimized.

Show comment Hide comment
@schneems

schneems Jul 21, 2012

Member

👍 on this or another method that does does not replace ::. I've run into this in the wild and was confused by the output.

@jeremy did you have an opion per @fxn's request?

Member

schneems commented Jul 21, 2012

👍 on this or another method that does does not replace ::. I've run into this in the wild and was confused by the output.

@jeremy did you have an opion per @fxn's request?

@fxn

This comment has been minimized.

Show comment Hide comment
@fxn

fxn Apr 17, 2013

Owner

This proposal hasn't got a lot of traction, and albeit makes sense, it will certainly break stuff.

It is a pity we often have to play safe with the inflector because of how brittle changes are, but backwards compatibility is important in this feature.

Maybe someday someone could write an alternative inflector gem that is supposed to accept all fixes and improvements :).

Owner

fxn commented Apr 17, 2013

This proposal hasn't got a lot of traction, and albeit makes sense, it will certainly break stuff.

It is a pity we often have to play safe with the inflector because of how brittle changes are, but backwards compatibility is important in this feature.

Maybe someday someone could write an alternative inflector gem that is supposed to accept all fixes and improvements :).

@fxn fxn closed this Apr 17, 2013

@balinterdi

This comment has been minimized.

Show comment Hide comment
@balinterdi

balinterdi Apr 18, 2013

Contributor

I see your point. As a sidenote, isn't a new major release (e.g 3.2.x -> 4.0) an opportunity to introduce non-backward compatible changes?

Contributor

balinterdi commented Apr 18, 2013

I see your point. As a sidenote, isn't a new major release (e.g 3.2.x -> 4.0) an opportunity to introduce non-backward compatible changes?

@fxn

This comment has been minimized.

Show comment Hide comment
@fxn

fxn Apr 18, 2013

Owner

If you are going to introduce them, it is a good place. Note however that many incompatible changes go through a transition phase of some sort, we try upgrading to be as smooth as possible (within the context of a major release).

Owner

fxn commented Apr 18, 2013

If you are going to introduce them, it is a good place. Note however that many incompatible changes go through a transition phase of some sort, we try upgrading to be as smooth as possible (within the context of a major release).

@steveklabnik

This comment has been minimized.

Show comment Hide comment
@steveklabnik

steveklabnik Apr 18, 2013

Member

Rails doesn't follow SemVer, 4.0 is intended to be as backwards compatible as possible.

Member

steveklabnik commented Apr 18, 2013

Rails doesn't follow SemVer, 4.0 is intended to be as backwards compatible as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment