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

use bufio.Reader instead of bufio.Scanner #79

Conversation

0tarof
Copy link
Contributor

@0tarof 0tarof commented Dec 21, 2021

gomockhandler check is failed when scan the large single line file.
Because os of hard limit of bufio.Scanner.

So use bufio.Reader to scan first line of file.

P.S.
There is another approach like filtering target files using extension, etc...

@sanposhiho
Copy link
Owner

Thanks for opening PR. I'll review it later.

Because os hard limit of bufio.Scanner.

I don't know about that. 😓

@0tarof
Copy link
Contributor Author

0tarof commented Dec 21, 2021

bufio.Scanner has default token size defained at bufio.MaxScanToken.
Ref: https://cs.opensource.google/go/go/+/refs/tags/go1.17.5:src/bufio/scan.go;l=81;bpv=1;bpt=1

if line length is greater than maxTokenSize, then Scanner.Scan function is failed with bufio.ErrTooLong.

In my case, gomockhandler scans large json configuration file on directory tree, then the command failed with that error.

Comment on lines 55 to 56
if cont {
for {
Copy link
Owner

Choose a reason for hiding this comment

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

Please change it to be simpler.

if cont {
 for {
// ...
 if !cont {
  break
 }
 }
}

for cont {
// ...
}

Copy link
Contributor Author

@0tarof 0tarof Dec 27, 2021

Choose a reason for hiding this comment

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

Thanks. I fixed it.

@0tarof 0tarof force-pushed the fix_check_failure_when_read_large_single_line_file branch from 6401812 to 6331707 Compare December 27, 2021 06:06
Copy link
Owner

@sanposhiho sanposhiho left a comment

Choose a reason for hiding this comment

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

@0tarof
Thanks for your contribution 👍
勉強になりました :)

@sanposhiho sanposhiho merged commit 7116642 into sanposhiho:master Jan 2, 2022
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