-
-
Notifications
You must be signed in to change notification settings - Fork 411
BSP fixes #5698
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
Merged
Merged
BSP fixes #5698
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There's no reason not to use it any more, now that --bsp-install works fine with a broken build too
The changed match is actually exhaustive. The changes here make that explicit.
Keep module instance around, so that we don't check the bspModulesById map twice, which limits the odds of discrepancies (in case of refactoring, …)
That request can be checked for all targets, not just a subset of them.
So that we have non-regression tests for it too
…ources Passing a build.mill file to buildTargetInverseSources used not to return anything. This changes that.
If a sources task depends on a compile one, a BSP-specific compile reporter needs to be around in that request too. This fixes that.
@alexarchambault could you try updating this line upstream and adding |
def goldenLiteralPrinter(x: Any): String = pprint.PPrinter.BlackWhite.apply(x).plainText |
Changes look good to me. |
This reverts commit 83ef7af.
lihaoyi
pushed a commit
to com-lihaoyi/utest
that referenced
this pull request
Aug 18, 2025
This makes golden testing not truncate test data when it's too large. See discussion in com-lihaoyi/mill#5698
@alexarchambault uTest 0.9.1 should have your fix |
If a sources task depends on a compile one, a BSP-specific compile reporter needs to be around in that request too. This fixes that.
Conflicts: integration/ide/bsp-server/resources/snapshots/logging
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes several small BSP issues, and tweaks the BSP snapshot tests too.
In particular, this makes the BSP snapshot tests more exhaustive, by testing a meta-build too, and making some tests more broad (main class request test).
This fixes issues with the buildTargetInverseSources request:
build.mill
file (while it doesn't return anything without the changes here)sources
task depends on acompile
one for example (as a check requires such a reporter to be around during all task evaluations in the BSP server, so that we don't miss any diagnostic)This PR commits can be reviewed one-by-one, and provide a bit of explanation of what each commit does in the commit message.