Add LineBufferSize option field #85
Conversation
To support configuring the line buffer size a new field LineBufferSize is added to the cmd.Options struct. This allows configuring the line buffer size to the callers needs instead of relying on the DEFAULT_LINE_BUFFER_SIZE.
cmd.go
Outdated
| // value DEFAULT_LINE_BUFFER_SIZE is usually sufficient, but if | ||
| // ErrLineBufferOverflow errors occur, try increasing the size with this | ||
| // field. | ||
| LineBufferSize int |
There was a problem hiding this comment.
Let's make it unit because it can't be negative (or zero). Then below, if options.LineBufferSize > 0 {.
There was a problem hiding this comment.
I'm unsure what you mean. uint instead or change the name of the field? 🙏
There was a problem hiding this comment.
I have updated the PR now. It does bring a, maybe only theoretical, implication. As OutputStream.SetLineBufferSize accept an int we need to cast it. Tihs could potentially bring an integer overflow as the range of uint is larger than int due to its unsigned nature. 🤔 WDYT, acceptable risk? Should we add a check?
|
Looks good. Re |
|
Released as v1.4.1. |
|
Thank you for your time in supporting this project 🙌🎉 |
To support configuring the line buffer size a new field
LineBufferSizeis addedto the
cmd.Optionsstruct. This allows configuring the line buffer size to thecallers needs instead of relying on the
DEFAULT_LINE_BUFFER_SIZE.This fixes #66 .