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

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dmartingarcia
Copy link

@dmartingarcia dmartingarcia commented Sep 27, 2022

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: rails/rails@179d0a1

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 and others added 2 commits September 27, 2022 17:09
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: rails/rails@179d0a1

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

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
Copy link

NormandLemay commented Jan 5, 2023

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
Copy link
Author

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

Changing it.

cc/ @kreintjes

@stanhu
Copy link

stanhu commented Jan 24, 2023

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

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.

4 participants