settingslogic icon indicating copy to clipboard operation
settingslogic copied to clipboard

Solve issues with aliases after upgrading to Psych ~> 4

Open dmartingarcia opened this issue 2 years ago • 4 comments

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

dmartingarcia avatar Sep 27 '22 15:09 dmartingarcia

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?

kreintjes avatar Dec 09 '22 14:12 kreintjes

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

NormandLemay avatar Jan 05 '23 15:01 NormandLemay

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

Changing it.

cc/ @kreintjes

dmartingarcia avatar Jan 05 '23 22:01 dmartingarcia

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

stanhu avatar Jan 24 '23 23:01 stanhu