Skip to content
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

BSP: do not filter clean-requests for meta-builds #2931

Merged
merged 1 commit into from
Jan 3, 2024

Commits on Dec 22, 2023

  1. fix: ensure the BSP clean works as expected

    Looking at the last change that was made to this code in
    com-lihaoyi@65fbd53
    it's not clear to me why this line was added where we map everything to
    `BspModule` and then filtering the build targets on that which where in
    the request. My assumption is that anything that the build client is
    requesting to clean _should_ be a valid identifier. If not, then that
    originally came from Mill, so I'm unsure why we have this. Before this
    change it was causing issues since not everything in `state.rootModules`
    was a `BspModule` so you'd get a match exception.
    
    To expand a bit the build targets here in an example `mill-test` (hello
    world) project are:
    
    ```
    BuildTargetIdentifier [
      uri = "file:///Users/<me>/Documents/scala-workspace/mill-test/milltest"
    ],BuildTargetIdentifier [
      uri = "file:///Users/<me>/Documents/scala-workspace/mill-test/mill-build"
    ]
    ```
    
    But when looking into the `state.rootModules` one of them is
    
    ```
    /Users/<me>/Documents/scala-workspace/mill-test
    ```
    
    This was the problematic one. I'm not sure if this is just a mapping
    issue or not, but the change I made instead just takes the build target
    identifies given from the client, does a look up in the
    `state.bspModulesById` mapping, and goes with it. This seems to work and
    simplifies this unless this is introducing an issue that I don't see.
    
    closes com-lihaoyi#2915
    ckipp01 committed Dec 22, 2023
    Configuration menu
    Copy the full SHA
    a18f1dd View commit details
    Browse the repository at this point in the history