binarylogic / settingslogic

A simple and straightforward settings solution that uses an ERB enabled YAML file and a singleton design pattern.
http://rdoc.info/projects/binarylogic/settingslogic
MIT License
1.4k stars 186 forks source link

Solve issues with aliases after upgrading to Psych ~> 4 #86

Open dmartingarcia opened 2 years ago

dmartingarcia commented 2 years ago

Aliases completely break the logic behind settingslogic gem after upgrading to the latest Psych version.

It has been described here: https://bugs.ruby-lang.org/issues/17866

I applied the same solution Rails team applies a time ago: https://github.com/rails/rails/commit/179d0a1f474ada02e0030ac3bd062fc653765dbe

It tries to call YAML module with the new arguments and failover over the old parameters interface

Psych diff between 3.3.2 and 4.0.0: https://my.diffend.io/gems/psych/3.3.2/4.0.0

kreintjes commented 1 year ago

Thanks for this fix @dmartingarcia, I ran into the same BadAlias issue. I tried your branch in my project (gem 'settingslogic', '~> 2.0', github: 'dmartingarcia/settingslogic', branch: 'patch-1'), but unfortunately it doesn't seem to work. I get the following error when trying to read settings:

[1] pry(main)> Brand.all
Settingslogic::MissingSetting: Missing setting 'file_contents' in 
from /.../ruby/gems/3.1.0/bundler/gems/settingslogic-31d455b29a11/lib/settingslogic.rb:189:in `missing_key'

Any idea what goes wrong?

NormandLemay commented 1 year ago

Yes the issue is:

def parse_yaml_content(file_content)
    YAML.load(ERB.new(file_contents).result, aliases: true).to_hash
  rescue ArgumentError
    YAML.load(ERB.new(file_contents).result).to_hash
  end

should be

def parse_yaml_content(file_content)
    YAML.load(ERB.new(file_content).result, aliases: true).to_hash
  rescue ArgumentError
    YAML.load(ERB.new(file_content).result).to_hash
end

file_content and not file_contents

dmartingarcia commented 1 year ago

@NormandLemay first of all, thanks a lot. a typo, my fault, you're right 🙈

Changing it.

cc/ @kreintjes

stanhu commented 1 year ago

Thanks for this! It doesn't look like this project is maintained. Should we fork this project somewhere else?