Skip to content

[BUG] Don't mutate passed configuration options#663

Merged
robin850 merged 2 commits intovmg:masterfrom
maschwenk:dont-mutate-options
Jan 26, 2019
Merged

[BUG] Don't mutate passed configuration options#663
robin850 merged 2 commits intovmg:masterfrom
maschwenk:dont-mutate-options

Conversation

@maschwenk
Copy link
Copy Markdown
Contributor

@maschwenk maschwenk commented Sep 21, 2018 •
edited
Loading

The markdown initializer mutates the options you pass it. I don't see any obvious reason why this is necessary?

Redcarpet::Markdown.new(Redcarpet::Render::HTML.new({ bizzle: 'zizzle' }.freeze), {})

RuntimeError: can't modify frozen Hash
from (pry):5:in `merge!'

Instead just merge and return a new hash 👍 👎 🤷‍♂️

@maschwenk maschwenk changed the title [BUG] Don't mutated passed options [BUG] Don't mutate passed configuration options Sep 21, 2018
@maschwenk
Copy link
Copy Markdown
Contributor Author

maschwenk commented Sep 28, 2018

@robin850 Just a friendly bump on this?

@maschwenk
Copy link
Copy Markdown
Contributor Author

maschwenk commented Dec 10, 2018

@robin850 another friendly 🙏 bump?

@robin850 robin850 merged commit 3100f65 into vmg:master Jan 26, 2019
robin850 added a commit that referenced this pull request Jan 26, 2019
@robin850
Tiny follow-up to #663
94f6e27
@robin850
Copy link
Copy Markdown
Collaborator

robin850 commented Jan 26, 2019

Sorry for the late answer ; thank you very much for the patch @maschwenk ! 🎉 I took the liberty to write a more explicit test for that in the aforementioned commit and give you credit in the changelog.

@maschwenk
Copy link
Copy Markdown
Contributor Author

maschwenk commented Jan 26, 2019

Haha great! I literally almost closed this yesterday because I was losing faith, glad I didn't :)

This was referenced Mar 11, 2021
Merged
Closed
Closed
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Merged
Open
@dependabot dependabot bot mentioned this pull request Mar 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@maschwenk @robin850