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

Overloading with nulls #13

Closed
davemitchell opened this issue Feb 24, 2011 · 5 comments
Closed

Overloading with nulls #13

davemitchell opened this issue Feb 24, 2011 · 5 comments
Assignees
Labels
Milestone

Comments

@davemitchell
Copy link

davemitchell commented Feb 24, 2011

I was expecting to be able to "unset" a property in an overriding config file. For example, consider the following:

In config/settings.yml
name: Admin
email: root@foo.com

In config/settings/development.yml
name: Nobody
email: null

However, it looks like instead of unsetting the value, the derived property is untouched.
That is, the above case yields:
Settings.name # => Nobody
Settings.email # => root@foo.com

Rather than what I was expecting:
Settings.name # => Nobody
Settings.email # => nil

Bug, or feature?

@tonyla
Copy link

tonyla commented Dec 15, 2011

The reason this is happening is this gem vendor'ed the deep_merge gem

vendor/deep_merge.rb:72
if source.nil? || (!source.is_a?(FalseClass) && source.respond_to?(:blank?) && source.blank?); return dest; end

As you can see it will just skip over any overrides that are false or blank. IMHO this is incorrect and has been fixed in the original gem located here.

https://github.com/danielsdeleo/deep_merge

I just added this dependency to my Gemfile and rails_config will just use the loaded gem instead of the vendored version.

@davemitchell
Copy link
Author

Cool, thanks!

@pkuczynski pkuczynski added the bug label Mar 31, 2014
@pkuczynski pkuczynski added this to the 0.4 milestone Mar 31, 2014
@toncid
Copy link

toncid commented Feb 19, 2015

Hello,

What's the status of this issue? It's still occurring and overriding with a blank value doesn't affect the original value.

Thanks!

@pkuczynski pkuczynski modified the milestones: 0.4, 1.3 May 12, 2016
@pkuczynski pkuczynski self-assigned this May 26, 2016
@pkuczynski
Copy link
Member

There is an open PR on deep merge gem: danielsdeleo/deep_merge#20

@pkuczynski
Copy link
Member

I made a mistake. In fact, danielsdeleo/deep_merge#33 solves this issue, not danielsdeleo/deep_merge#20

@pkuczynski pkuczynski modified the milestones: 1.4, 1.6.0 Oct 23, 2017
@pkuczynski pkuczynski modified the milestones: 1.6.0, 1.6.1, 1.6.2 Nov 3, 2017
jrafanie added a commit to jrafanie/config that referenced this issue Jan 31, 2018
jrafanie added a commit to jrafanie/config that referenced this issue Jan 31, 2018
jrafanie added a commit to jrafanie/config that referenced this issue Jan 31, 2018
jrafanie added a commit to jrafanie/config that referenced this issue Feb 7, 2018
Previously, if you merged in {a: nil}, it would not overwrite an
existing value.  In rubyconfig#13, this was found to be an undesirable default
behavior.

Now, we will merge nil values by default. You can retain the old
behavior via `config.merge_nil_values = false` in your Config initializer.

This required changes to deep merge, found in: danielsdeleo/deep_merge#33
This was released in deep_merge 1.2.0+.

Fixes rubyconfig#13
jrafanie added a commit to jrafanie/config that referenced this issue Feb 7, 2018
Previously, if you merged in {a: nil}, it would not overwrite an
existing value.  In rubyconfig#13, this was found to be an undesirable default
behavior.

Now, we will merge nil values by default. You can retain the old
behavior via `config.merge_nil_values = false` in your Config initializer.

This required changes to deep merge, found in: danielsdeleo/deep_merge#33
This was released in deep_merge 1.2.0+.

Fixes rubyconfig#13
jrafanie added a commit to jrafanie/config that referenced this issue Feb 7, 2018
Previously, if you merged in {a: nil}, it would not overwrite an
existing value.  In rubyconfig#13, this was found to be an undesirable default
behavior.

Now, we will merge nil values by default. You can retain the old
behavior via `config.merge_nil_values = false` in your Config initializer.

This required changes to deep merge, found in: danielsdeleo/deep_merge#33
This was released in deep_merge 1.2.0+.

Fixes rubyconfig#13
jrafanie added a commit to jrafanie/config that referenced this issue Feb 7, 2018
Previously, if you merged in {a: nil}, it would not overwrite an
existing value.  In rubyconfig#13, this was found to be an undesirable default
behavior.

Now, we will merge nil values by default. You can retain the old
behavior via `config.merge_nil_values = false` in your Config initializer.

This required changes to deep merge, found in: danielsdeleo/deep_merge#33
This was released in deep_merge 1.2.0+.

Fixes rubyconfig#13
jrafanie added a commit to jrafanie/config that referenced this issue Feb 7, 2018
Previously, if you merged in {a: nil}, it would not overwrite an
existing value.  In rubyconfig#13, this was found to be an undesirable default
behavior.

Now, we will merge nil values by default. You can retain the old
behavior via `config.merge_nil_values = false` in your Config initializer.

This required changes to deep merge, found in: danielsdeleo/deep_merge#33
This was released in deep_merge 1.2.0+.

Fixes rubyconfig#13
jrafanie added a commit to jrafanie/config that referenced this issue Feb 7, 2018
Previously, if you merged in {a: nil}, it would not overwrite an
existing value.  In rubyconfig#13, this was found to be an undesirable default
behavior.

Now, we will merge nil values by default. You can retain the old
behavior via `config.merge_nil_values = false` in your Config initializer.

This required changes to deep merge, found in: danielsdeleo/deep_merge#33
This was released in deep_merge 1.2.0+.

Fixes rubyconfig#13
jrafanie added a commit to jrafanie/config that referenced this issue Feb 7, 2018
Previously, if you merged in {a: nil}, it would not overwrite an
existing value.  In rubyconfig#13, this was found to be an undesirable default
behavior.

Now, we will merge nil values by default. You can retain the old
behavior via `config.merge_nil_values = false` in your Config initializer.

This required changes to deep merge, found in: danielsdeleo/deep_merge#33
This was released in deep_merge 1.2.0+.

Fixes rubyconfig#13
@pkuczynski pkuczynski modified the milestones: 1.7.0, 1.7.1 Feb 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants