nytimes / Fech

Deprecated. Please see https://github.com/dwillis/Fech for a maintained fork.
http://nytimes.github.io/Fech/
Other
115 stars 30 forks source link

Read filing's contents using specified encoding #86

Open myersjustinc opened 5 years ago

myersjustinc commented 5 years ago

The @encoding instance variable on a Filing object is ignored in methods such as Filing#form_type, which can lead to an ArgumentError ("invalid byte sequence in UTF-8"). Before the included change in lib/fech/filing.rb is made, the included test case demonstrates such an error when we try to call Filing#summary.

This change takes @encoding into account when reading the filing from disk, which avoids the ArgumentError.

(The entire test suite now only has two failing tests, both of which are addressed in #83 and don't appear to be related to this specific change.)

dwillis commented 5 years ago

@myersjustinc Thanks for this! I am not sure if the folks at the NYT are still maintaining this, but I've got a fork here and would be happy to have this PR there.

myersjustinc commented 5 years ago

Sounds good. Once I've filed a matching PR on dwillis/Fech, should I also leave this one open on this repo, or should I close this one to avoid confusion?

dwillis commented 5 years ago

I would leave it open, as I'm unsure whether this repo is actively monitored.