add ability to override execAsync's maxBuffer default of 20MiB #284
Conversation
|
Marking this as an update of #270 (which was closed). |
|
Does this also work for |
|
Good catch - option added to execSync as well. |
|
Do we also need to make the change on lines like https://github.com/pieterjandesmedt/shelljs/blob/exec-maxBuffer-override/src/exec.js#L90 where the value is hardcoded? Also, could you move |
|
Done & done. |
|
Let me check this later today, and I'll give you an LGTM if I don't find any issues. do you know of any way that we could continue using small buffers but simply open up a new buffer when it gets full? This would be even better, since then we could have an option to effectively never crash from filling up the buffer |
|
AFAIK, there's no easy way to increase the buffer size once the child process is running... |
There was a problem hiding this comment.
Is there a way to combine these checks into one if? That way we only have to assign the default value once, which is probably cleaner.
|
Overall, this looks great! Just a few comments above to address. I tested this on Linux, and saw that this helped avoid crashes when the output buffer was exceeded, so it works as intended. Also, please squash your commits and rebase off the upstream master, and this should be good to merge. |
|
@pieterjandesmedt: Could you add the changes and rebase onto master please? I'd really like to get this into the next release. |
|
@pieterjandesmedt: bump |
add maxBuffer override to execSync as well added constant DEFAULT_MAXBUFFER_SIZE and made sure maxBuffer is an integer fixed indentation and removed delimiter in example add ability to override execAsync's maxBuffer default of 20MiB
pieterjandesmedt
force-pushed
the
exec-maxBuffer-override
branch
from
699c6d8 to
5d235bd
Compare
|
Unsure how to proceed... I squashed the commits an created PR #333. You decide :-) |
|
@pieterjandesmedt: As much as I hate to reject a PR, it looks like this has been fixed by #335, so I have to close this. Sorry. |
|
No problem, as long as it's fixed, I'm happy :-) |
3 participants
exec() maxBuffer override option now in a separate branch.