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

Stream input files instead of reading entire contents #61

Merged
merged 2 commits into from
Feb 14, 2019

Conversation

chancez
Copy link
Collaborator

@chancez chancez commented Feb 14, 2019

Also supports streaming input from stdin and removes the requirement for
of passing any files to faq, defaulting to stdin if no file args are
specified.

I removed the usage of linguist because it's not really helping and I felt my custom code to detect xml/yaml/json is sufficient, along with the existing filename extension based detection. We could continue to use linguist, but it would mean we need to read a lot more of the file's contents (maybe all of it?) and pass those contents to lingust, which makes things complicated since the bufio.Reader buffer is of limited length.

@chancez chancez force-pushed the stream_file_inputs branch 3 times, most recently from 9c28ef4 to 9c97587 Compare February 14, 2019 01:57
Chance Zibolski added 2 commits February 14, 2019 10:00
Currently these decoders are not using a streaming decoder so we read
everything at once using ioutil.ReadAll, which means these do not return
io.EOF as the faq code expects a decoder to return.
Also supports streaming input from stdin and removes the requirement for
of passing any files to faq, defaulting to stdin if no file args are
specified.
@chancez
Copy link
Collaborator Author

chancez commented Feb 14, 2019

Okay, I fixed the review comments and added back linguist detection. I rebased on top of #62 because I found a bug in decoding logic while handling this and wanted to fix that independently of this PR.

@jzelinskie jzelinskie merged commit 9769052 into jzelinskie:master Feb 14, 2019
@chancez chancez deleted the stream_file_inputs branch February 14, 2019 19:11
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.

2 participants