ckdake/setler

setting something to false returns true

Closed this issue · 4 comments

It works when false is set in the defaults, but not if it is set at runtime.

I think this is the issue (in settings.rb):

def self.[](var)
  the_setting = thing_scoped.find_by_var(var.to_s)
  the_setting.try(:value) || @@defaults[var]
end

I'm not sure how you want to deal with nil values, should setting it to nil mean that it goes back to default? If so, this would be correct:

def self.[](var)
  the_setting = thing_scoped.find_by_var(var.to_s)
  val = the_setting.try(:value)
  val.nil? ? @@defaults[var] : val
end

I will submit a patch with tests, but I think the README should be explicit about the behavior. Also, changing that would be a breaking change for some people, I imagine (though I think only people who were depending on setting something false returning true). Let me know! For now, I will just patch it in my Settings class. :)

Looking forward to your patch! A test will be needed to get it merged, but I think this is a good thing to fix.

yeah i was actually thinking about it last night and i think it should be a little different.

def self.[](var)
  the_setting = thing_scoped.find_by_var(var.to_s)
  the_setting.present? ? the_setting.value : @@defaults[var]
end

this would mean that the only way to revert to your default would be to actually remove the setting. this seems like the most straightforward behavior. setting it to nil should probably just set it to nil.

i will try to get a patch in sometime today--wanted to get your input before going through the trouble. :)

Agreed, setting it to nil should probably set it to nil instead of resetting it to defaults

closing with pull request #12