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

"Import External Q# Package" VS Code command palette command #2079

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sezna
Copy link
Contributor

@sezna sezna commented Dec 27, 2024

This PR formalizes the "registry" md document into a JSON file, and then adds a command palette command which consumes it. This will hopefully increase discoverability of the project system, and formalizes our listing of known libraries a bit more.

Screen.Recording.2024-12-27.at.10.18.05.AM.mov

@sezna sezna marked this pull request as draft December 30, 2024 22:13
@sezna sezna marked this pull request as ready for review December 31, 2024 21:05
Comment on lines +296 to +307
const projectChoices: Array<{ name: string; ref: LocalProjectRef }> = [];

projectFiles.forEach((file) => {
const dirName = file.path.slice(0, -"/qsharp.json".length);
const relPath = getRelativeDirPath(qsharpJsonDir.path, dirName);
projectChoices.push({
name: dirName.slice(dirName.lastIndexOf("/") + 1),
ref: {
path: relPath,
},
});
});
Copy link
Member

Choose a reason for hiding this comment

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

Would look more natural to write this as a .map() in my opinion.

JSON.stringify(manifestObj, null, 2),
);
if (!(await vscode.workspace.applyEdit(edit))) {
vscode.window.showErrorMessage(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
vscode.window.showErrorMessage(
await vscode.window.showErrorMessage(

const dirName = file.path.slice(0, -"/qsharp.json".length);
const relPath = getRelativeDirPath(qsharpJsonDir.path, dirName);
projectChoices.push({
name: dirName.slice(dirName.lastIndexOf("/") + 1),
Copy link
Member

Choose a reason for hiding this comment

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

I can't find any bugs from just reading the code, but the / separator has me wondering if this has been tested on Windows (desktop and web)? LMK if you want me to give it a spin on my laptop.

-- in my experience, Windows browser, Windows desktop, and linux/mac browser/desktop all have unique path handling concerns. And even just in the browser: the playground, the test FS, and the local FS all show up as different virtual filesystems with different quirks. So I've become very distrustful of filesystem code... forgive me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, this is previously-existing code from Bill's implementation, I just moved it elsewhere. It's been live for a while now, and I believe all paths in the vscode APIs are normalized -- I can't find the exact docs but this hints at it https://github.com/microsoft/vscode/blob/main/src/vscode-dts/vscode.d.ts#L1466

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

Successfully merging this pull request may close these issues.

2 participants