Skip to content
This repository has been archived by the owner on Mar 6, 2019. It is now read-only.

Read filing's contents using specified encoding #86

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

myersjustinc
Copy link

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.)

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"). 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`.
@dwillis
Copy link
Contributor

dwillis commented Feb 7, 2019

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

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

dwillis commented Feb 7, 2019

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants