Skip to content

Conversation

@danielfone
Copy link
Contributor

@danielfone danielfone commented Jul 3, 2016

Currently the #form_group method attempts to mutate the
:control_col option if no label is supplied. Since this option is
passed by the user, it may be a frozen string literal. Instead, we
simply avoid mutating the supplied string.

As we approach Ruby 3 and frozen string literals become more common,
this is more likely to fail.

*edit: updated changelog.

Currently the `#form_group` method attempts to mutate the
`:control_col` option if no label is supplied. Since this option is
passed by the user, it may be a frozen string literal. Instead, we
simply avoid mutating the supplied string.

As we approach Ruby 3 and frozen string literals become more common,
this is more likely to fail.
@danielfone danielfone force-pushed the fix-string-mutation-in-form-group branch from 96e9984 to 6eba354 Compare July 3, 2016 23:28

if get_group_layout(options[:layout]) == :horizontal
control_class = (options[:control_col] || control_col.clone)
control_class = options[:control_col] || control_col
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This offers a slight memory improvement since it's only creating new string objects as needed, instead of cloning every time.

@mattbrictson
Copy link
Contributor

Nice improvement. Thanks! 🙇

@mattbrictson mattbrictson merged commit 2194198 into bootstrap-ruby:master Jul 4, 2016
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.

2 participants