-
Notifications
You must be signed in to change notification settings - Fork 455
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
Compiler log unterminated on no-op build. #5545
Comments
I suppose https://github.com/rescript-lang/ninja/pull/2/files is more relevant? (it changed how the |
Thanks -- indeed. |
Fixes https://github.com/rescript-lang/rescript-compiler/issues/5543 Commit: ed4d8abfb80826b73501f035ee01b1f2b15a9f53
In fact, it seems unrelated to ninja, but related to the vscode extension. |
CC @zth could you take a look -- this looks like an issue with the current extension. |
On it. |
I draw the same conclusions as the initial post - the compiler log is not terminated for some reason, so the extension does not clear errors. I also tested with ReScript |
Looks like both the compiler and the extension have changed behaviour in their current versions. Was wondering whether the extension should be resilient to the compiler not printing the log terminator, just as the previous version of the extension seems to be doing. Though the normal situation where that would happen is when the compiler is in the process of building, and has not finished yet. I guess this is possible as the compiler log would be written incrementally to disk and the watcher could pick up an incomplete change. So perhaps making the extension more robust w.r.t. unterminated output would also make it wrong. |
So: isolated a more basic wrong behaviour, without using a watcher: |
That's easy to add to a test too, which would be much more complicated if a watcher were involved. |
Taking a less rushed look at ninja now. |
Actually all of the above. |
1
rescript build -w
2 introduce error
3 fix error
end up with:
This shows up in issues not cleared in the editor extension.
FYI @zth
The text was updated successfully, but these errors were encountered: