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

Breaking changes in 0.3 #32

Open
axelson opened this issue Oct 12, 2018 · 4 comments
Open

Breaking changes in 0.3 #32

axelson opened this issue Oct 12, 2018 · 4 comments

Comments

@axelson
Copy link

axelson commented Oct 12, 2018

It looks like there's some breaking changes in 0.3. I haven't looked into them too fully yet but some of my tests are now breaking. Here's an example FunctionClauseError I'm getting.

     ** (FunctionClauseError) no function clause matching in Regex.replace/4

     The following arguments were given to Regex.replace/4:

         # 1
         ~r/['’]s/u

         # 2
         nil

         # 3
         "s"

         # 4
         [global: nil]

     Attempted function clauses (showing 2 out of 2):

         def replace(regex, string, replacement, options) when is_binary(string) and is_binary(replacement) and is_list(options)
         def replace(regex, string, replacement, options) when is_binary(string) and is_function(replacement) and is_list(options)

     code: conn = post conn, api_community_path(conn, :create), %{community: community_params}
     stacktrace:
       (elixir) lib/regex.ex:595: Regex.replace/4
       (slugger) lib/slugger.ex:40: Slugger.slugify/2

Is there any documentation of the breaking changes?

@ssbb
Copy link

ssbb commented Nov 26, 2018

Have an issue with this character too. README promises only A-Za-z0-9 alphabet is possible, but it's not:

iex(2)> Slugger.slugify_downcase "Children´s Playroom"
"children´s-playroom"
iex(3)> 

@axelson
Copy link
Author

axelson commented Nov 26, 2018

For comparison here is the correct output from version 0.2.0:

iex(2)> Slugger.slugify_downcase "Children´s Playroom"
"children-s-playroom"

@axelson
Copy link
Author

axelson commented Jan 30, 2019

@h4cc any chance you could take a look at this? Would you accept a PR to bring back the old behavior?

@axelson
Copy link
Author

axelson commented Jan 30, 2019

It looks like this is the change that broke things: 570c95e

mjankowski added a commit to thoughtbot/constable that referenced this issue Feb 1, 2019
We were seeing app errors occur when people tried to create announcements where
the title contained an Emoji.

Example: https://app.honeybadger.io/projects/48229/faults/44121036#notice-summary

We tracked this back to the Announcement changeset massaging a `slug` value out
of the announcement title while it saves. We use the Slugger library to do this,
which has a bug in version 0.3.0 around removing chars outside the expected
range: h4cc/slugger#32

A side effect of this was that our title values wound up with non-UTF8 chars in
them and postgres threw an error.

This change locks the version of Slugger to one which was working, and adds test
coverage for including Emoji in announcement titles.
mjankowski added a commit to thoughtbot/constable that referenced this issue Feb 1, 2019
We were seeing app errors occur when people tried to create announcements where
the title contained an Emoji.

Example: https://app.honeybadger.io/projects/48229/faults/44121036#notice-summary

We tracked this back to the Announcement changeset massaging a `slug` value out
of the announcement title while it saves. We use the Slugger library to do this,
which has a bug in version 0.3.0 around removing chars outside the expected
range: h4cc/slugger#32

A side effect of this was that our title values wound up with non-UTF8 chars in
them and postgres threw an error.

This change locks the version of Slugger to one which was working, and adds test
coverage for including Emoji in announcement titles.
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

No branches or pull requests

2 participants