Skip to content

Capitalize does not follow Shopify spec #326

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

Closed
microalps opened this issue Mar 30, 2021 · 11 comments
Closed

Capitalize does not follow Shopify spec #326

microalps opened this issue Mar 30, 2021 · 11 comments

Comments

@microalps
Copy link

microalps commented Mar 30, 2021

Note: I am not a user of liquidjs but found this while working on dotliquid/dotliquid#390

Shopify is working on updating their docs (see Shopify/liquid#1403 and the future docs at https://ericfromcanada.github.io/liquid/filters/capitalize/) that notes that capitalize actually lower cases everything. So mcDonald becomes Mcdonald! LiquidJS does not implement this way (IMHO is a more sensible version).

I tested Ruby liquid on my server -

{
  "template": "{{ name | capitalize }}",
  "data": {
    "name": "John G. Chalmers-Smith"
  }
}

Result: John g. chalmers-smith
if I add a space before the name, it will be all lowercase.

FYI - In Rails, the equivalent to this is named upcase_first. See https://glaucocustodio.github.io/2016/05/19/rails-5-new-upcase-first-method/ and https://stackoverflow.com/questions/3724913/how-to-capitalize-the-first-letter-in-a-string-in-ruby

@daviburg
Copy link

🤦‍♂️

@daviburg
Copy link

I dislike it, but it appears we need to offer an option for compatibility on the behavior of this filter, whether customers need the Shopify behavior or the LiquidJS behavior.

@microalps
Copy link
Author

The first step, I think, is somehow having an alternate that DOES what we are doing as a filter across liquid implementations. If upcase_first is it - so be it. If we can coordinate that with Shopify Liquid, liquidjs, and DotLiquid, even better.
We should definitely document somewhere that capitalize diverges from the standard - and users should use the above (even if the behavior is the same at the moment).

Somewhere down the line, liquidjs and DotLiquid can each individually decide how to offer opt-in for this breaking change to the capitalize. Without an alternative though, nobody will opt-in IMHO the "standard" is very ruby specific.

@daviburg
Copy link

The problem with alternate filter names is that a customer coming from one implementation to another cannot just opt-in compatibility option from its origin, he would instead need to modify its template.

@microalps
Copy link
Author

Clarification on what I meant:
alternate filter name - capitalizes just the first character and leaves the rest alone (ruby/rails calls this upcase_first). This is apparently the current 'capitalize' behavior in liquidjs, dotliquid 2.1, scriban...
capitalize - needs to stay the same as whatever each implementation has today unless they opt-in for the Ruby version.

So existing liquidjs users won't opt in - until they switch all their references to the alternate filter name. This will allow them time to ease into this (both filters do the same thing without the opt-in)
Customers coming from Ruby implementation will opt-in for the standard and capitalize will work for them as they are used to on ruby.

I'm all ears to hearing additional solutions, but we need to coordinate, that's the key in my opinion.

@microalps
Copy link
Author

I also suggest rounding out the 'capitalization' options with a CSS text-transform:capitalize equivalent - upcase_words or something.

@microalps
Copy link
Author

@harttle Do you have any thoughts on this topic?

@harttle
Copy link
Owner

harttle commented Apr 8, 2021

Since it's not consistent with Shopify/liquid, I regard it as a bug. It'll be fixed on the next patch version.

@microalps
Copy link
Author

Do you have any plans or thoughts how to support users that are currently using Capitalize filter? Will you offer a non-standard filter for them to switch to? A compatibility flag? Both?

@harttle
Copy link
Owner

harttle commented Apr 8, 2021

I think it's purely a bug. The proposed/needed capitalize filter is not standard and shouldn't be builtin. Plus it's easy to implement, I'd rather leave it to users.

@harttle harttle added the bug label Apr 16, 2021
@harttle harttle changed the title DISCUSSION: Capitalize does not follow Shopify spec Capitalize does not follow Shopify spec Apr 16, 2021
harttle pushed a commit that referenced this issue Apr 17, 2021
## [9.23.4](v9.23.3...v9.23.4) (2021-04-17)

### Bug Fixes

* capitalize filter not lower case trailing string, fixes [#326](#326) ([6548765](6548765))
@harttle
Copy link
Owner

harttle commented Apr 17, 2021

🎉 This issue has been resolved in version 9.23.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

3 participants