Add encoding options to Resolver::Static#read and Resolver::Dynamic#read

[denzelem]

PR for #176.

[brenogazzola]

Why is the parameter here named options instead of encoding like in assets.rb and static.rb?

[denzelem]

I used an option hash to avoid duplicating the default value to this top-level method.

Alternative 1:

def read(logical_path, encoding: 'ASCII-8BIT')
  # ...
  asset.content(encoding: encoding)
  # ...
end

Alternative 2:

def read(logical_path, encoding: nil)
  # ...
  options = {}
  options[:encoding] = encoding if encoding.present?
  asset.content(**options)
  # ...
end

@brenogazzola Wdyt?

[dhh]

Don't see how this works when the asset is a binary file? If we're assuming ASCII-8BIT, what happens when you try to read a PNG?

[denzelem]

Don't see how this works when the asset is a binary file? If we're assuming ASCII-8BIT, what happens when you try to read a PNG?

@dhh Do you think that File.binread(path) is different to File.read(path, encoding: 'ASCII-8BIT')? My assumption was, that binread (rb_ascii8bit_encoding C-Method) reads the bytes with the encoding ASCII-8BIT.

So reading an PNG image should be no breaking change with this PR.

require 'open-uri'
require 'tmpdir'

image_url = 'https://picsum.photos/id/237/200/300'

Dir.mktmpdir do |dir|
  image_path = File.join(dir, 'example.jpg')

  URI.open(image_url) do |image|
    File.open(image_path, 'wb') do |file|
      file.write(image.read)
    end
  end

  puts 'Encoding binread ' + File.binread(image_path).encoding.to_s
  puts 'Encoding read with ASCII-8BIT ' + File.read(image_path, encoding: 'ASCII-8BIT').encoding.to_s
end

exit

Encoding binread ASCII-8BIT
Encoding read with ASCII-8BIT ASCII-8BIT

[theodorton]

@dhh @brenogazzola I don't see any blockers for merging this - what do you think?