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.

Sign up for GitHub

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

Jump to bottom

implemented a new feature that Hashie::Mash.load accepts a Pathname object #331 #336

Closed
wants to merge 3 commits into from
Closed

implemented a new feature that Hashie::Mash.load accepts a Pathname object #331 #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc11edf
implemented a new feature that Hashie::Mash.load accepts a Pathname o…
gipcompany Dec 25, 2015
1f2a551
fix RuboCop failures
gipcompany Dec 25, 2015
362826e
fix the following Argumenterror
gipcompany Dec 25, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/hashie/extensions/parsers/yaml_erb_parser.rb
View file
Open in desktop
Expand Up @@ -11,6 +11,7 @@ def initialize(file_path)

def perform
template = ERB.new(@content)
@file_path = @file_path.to_s if @file_path.respond_to? :to_s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right place to do this, if perform is called over and over again it's wasteful, and it's modifying the state of the object which feels like a side effect. Furthermore if I pass some junk that responds to to_s here we'll just try to load that, and that doesn't seem safe.

Ideally this should be directly supported by YAML.load, but short of that I would do this in initialize and more explicitly.

@file_path = file_path.is_a?(Pathname) ? file_path.to_s : file_path

template.filename = @file_path
YAML.load template.result
end
Expand Down
15 changes: 15 additions & 0 deletions spec/hashie/parsers/yaml_erb_parser_spec.rb
View file
Open in desktop
@@ -1,6 +1,21 @@
require 'spec_helper'

describe Hashie::Extensions::Parsers::YamlErbParser do
describe '#perform' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already a describe '.perform' below, so combine them. Something like this:

describe '.perform' do
   context 'a file' do
   end
   context 'Pathname' do
   end
end

let(:tempfile) do
file = Tempfile.new(['foo', '.yml'])
file.write("---\nfoo: hello\n")
file.rewind
file
end

subject { described_class.new(Pathname tempfile.path) }

it '"#perform" can be done in case of path is a Pathname object.' do
expect(subject.perform).to eq 'foo' => 'hello'
end
end

describe '.perform' do
let(:config) do
<<-EOF
Expand Down