Fix notebook target profile to default to Unrestricted instead of workspace setting#2526
Fix notebook target profile to default to Unrestricted instead of workspace setting#2526
Conversation
…kspace setting Co-authored-by: minestarks <16928427+minestarks@users.noreply.github.com>
language_service/src/state.rs
Outdated
| .unwrap_or_default(), | ||
| }; | ||
| let configuration = merge_configurations(¬ebook_configuration, &configuration); | ||
| let configuration = merge_configurations_for_notebook(¬ebook_configuration, &configuration); |
There was a problem hiding this comment.
this feels like overkill - couldn't you just have the notebook_metadata.target_profile default to Unrestricted above if it's not specified?
Actually, maybe just look at whether the VS Code layer (who constructs this notebook metadata) should just pass in Unrestricted as a default (if you go that route you can probably just revert your rust changes). Since really it's just VS Code business logic we're talking about here, so that deafult belongs there. You may want to add a VS Code integration test for that if you go that route
There was a problem hiding this comment.
You're absolutely right, that approach is much cleaner. I've moved the default to the VS Code layer instead - now getQSharpConfigMetadata() returns { targetProfile: "unrestricted" } by default rather than an empty object. This eliminates the need for the separate Rust function and keeps the business logic where it belongs. Also added a VS Code integration test as suggested. Changes committed in de7e95e.
Co-authored-by: minestarks <16928427+minestarks@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
# Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
… helpers Co-authored-by: minestarks <16928427+minestarks@users.noreply.github.com>
|
Refresh your PR description since you changed your approach completely from when you started |
Updated the PR description to reflect the current VS Code layer approach instead of the original Rust implementation. The description now accurately describes the final solution where |
Currently, the language service defaults notebooks to the workspace configuration target profile, which can cause incorrect errors when the workspace is set to a restrictive profile like "base". This creates a mismatch with the Python runtime behavior where notebooks should use unrestricted operations by default.
getQSharpConfigMetadata()to return{ targetProfile: "unrestricted" }by defaultProblem
When a user sets their workspace target profile to "base" and opens a Jupyter notebook with Q# code that uses unrestricted operations (like measurement results in conditionals), the language service incorrectly shows errors:
Solution
Modified the VS Code notebook configuration logic to default to
"unrestricted"target profile when no explicit configuration is provided viaqsharp.init(), rather than falling back to the workspace configuration.Changes
getQSharpConfigMetadata()invscode/src/language-service/notebook.tsto return{ targetProfile: "unrestricted" }when no Q# config metadata is foundnotebook.test.tsto verify notebooks default to unrestricted profile behaviortest-unrestricted-default.ipynbthat contains unrestricted operations without explicit configurationTesting
"Notebook defaults to unrestricted target profile"to verify the fixThis ensures notebooks behave consistently with Python runtime behavior while still allowing explicit target profile configuration when needed via
qsharp.init().Fixes #2525.