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
Split by lines when parsing stdout of build #1276
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -735,9 +735,13 @@ export class ArduinoApp { | |
} | ||
return ret; | ||
} | ||
let stdoutbuf = ""; | ||
const stdoutcb = (line: string) => { | ||
if (cocopa.callback) { | ||
cocopa.callback(line); | ||
stdoutbuf += line; | ||
let lines = stdoutbuf.split('\n'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
stdoutbuf = lines.pop()!; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This statement can be removed, there is no point in updating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The stdoutbuf is upvalue variable of the closure, the last update is taking the remain characters that in last line. |
||
lines.forEach((line) => cocopa.callback(line)); | ||
} | ||
if (verbose) { | ||
arduinoChannel.channel.append(line); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name
line
is obviously misleading since it really is chunks of stdout text, not specifically split into lines, so this name should be updated as well.I suggest
stdoutText
which is descriptive and also matches the documentation to thespawn
function in src/common/util.ts (which is missing an update to match its current parameters, great if you could fix that as well).With a rename of the function argument in
stdoutcb
(and fixstderrcb
as well at the same time), then you could usestdoutText
directly without needing to copy to astdoutbuf
variable.