Skip to content
This repository was archived by the owner on Oct 1, 2024. It is now read-only.

Fix line-oriented callbacks #1494

Merged
merged 3 commits into from
Apr 29, 2022
Merged

Fix line-oriented callbacks #1494

merged 3 commits into from
Apr 29, 2022

Conversation

nya3jp
Copy link
Contributor

@nya3jp nya3jp commented Apr 16, 2022

stdoutcb/stderrcb assume that an input string is a single line, but in fact it can contain multiple lines, or even be a part of a line.

One of the consequence is that cocopa fails to parse compiler output when it contains long lines (#1275).

This patch fixes the issue by introducing a buffer to accumulate received data before passing them to callbacks.

Fixes #1275.

stdoutcb/stderrcb assume that an input string is a single line, but
in fact it can contain multiple lines, or even be a part of a line.

One of the consequence is that cocopa fails to parse compiler output
when it contains long lines (microsoft#1275).

This patch fixes the issue by introducing a buffer to accumulate
received data before passing them to callbacks.

Fixes microsoft#1275.
@ghost
Copy link

ghost commented Apr 16, 2022

CLA assistant check
All CLA requirements met.

@nya3jp
Copy link
Contributor Author

nya3jp commented Apr 20, 2022

I'm checking with my employer if I can sign the CLA.

@nya3jp
Copy link
Contributor Author

nya3jp commented Apr 20, 2022

Signed CLA.

@benmcmorran
Copy link
Member

/azp run

Copy link
Member

@benmcmorran benmcmorran left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @nya3jp! I pushed one small change to avoid n^2 runtime on long lines, but otherwise this looks good to me. Any more comments you may see are just me convincing our internal CI systems to test this PR.

@benmcmorran
Copy link
Member

/azp run

@benmcmorran benmcmorran merged commit b844da1 into microsoft:main Apr 29, 2022
@nya3jp
Copy link
Contributor Author

nya3jp commented Apr 30, 2022

@benmcmorran I overlooked the O(n^2) issue, thanks for catching it!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parse failed when cmdline too long
2 participants