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.

Sign up for GitHub

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

Lazy activation for most commands, jres view #150

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

Lazy activation for most commands, jres view #150

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2c80b2b
check for pxt.json anywhere in ws folder
jwunderl Mar 17, 2023
d8b2ab3
git diff
jwunderl Mar 18, 2023
b793807
hide asset types, commands until a makecode project is actually loaded
jwunderl Mar 18, 2023
265c762
only include context menu sim when in mkcd project, move to debug gro…
jwunderl Mar 21, 2023
bc3da04
Merge branch 'main' of https://github.com/microsoft/pxt-vscode-web in…
jwunderl Mar 30, 2023
a7da5af
add condition on makecode.testBlocks command
jwunderl Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 41 additions & 9 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@
"url": "https://github.com/microsoft/vscode-makecode"
},
"activationEvents": [
"onCommand:makecode.create",
"workspaceContains:./pxt.json",
Copy link
Member

Choose a reason for hiding this comment

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

I'm not actually sure this is a valid glob pattern. Doesn't seem to work for me looking at a glob checker online. Might be why it was activating too much before.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I wasn't completely sure what was going on here, was planning on looking into it a bit more and possibly filing a bug; I tried the three variants of 'top level file checking' (workspaceContains:./pxt.json, workspaceContains:/pxt.json, workspaceContains:pxt.json) and saw the same behavior for each, and it seems like they should have worked (or at least the last one) as I see similar checks in other extensions activation events; e.g. https://github.com/microsoft/vscode-python/blob/main/package.json#L71-L77 (and if it's an invalid pattern I would think it would want to throw an error or default to false?)

That said, I did forget to mention that this still doesn't fully fix the issue as we still appear to be getting hit by the bug mentioned in #147 (comment) and activating early for that treason at times. (that's what I get for putting up a draft pr late on a friday) -- it does minimize it a bit though as fewer extra commands would show up.

"workspaceContains:**/pxt.json",
Copy link
Member

Choose a reason for hiding this comment

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

Would you still need makecode.create as an activation event here? From my (small) knowledge that command should still be an activation command options since you'd be running it with nothing but an empty workspace open.

Copy link
Member Author

Choose a reason for hiding this comment

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

In vscode that line was giving a warning saying it was now unneeded and is taken as an activation event automatically since we define the command

Copy link
Member

Choose a reason for hiding this comment

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

Nice. That used to be a common extension issue (forgetting to add a command activation) so that makes sense for core vscode to have added. Can't run a command if you are not activated.

"onFileSystem:mkcdfs"
],
"browser": "./dist/web/extension.js",
Expand Down Expand Up @@ -174,27 +173,32 @@
{
"id": "imageExplorer",
"name": "%makecode.imageExplorer.title%",
"icon": "media/logo.svg"
"icon": "media/logo.svg",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"id": "animationExplorer",
"name": "%makecode.animationExplorer.title%",
"icon": "media/logo.svg"
"icon": "media/logo.svg",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"id": "tileExplorer",
"name": "%makecode.tileExplorer.title%",
"icon": "media/logo.svg"
"icon": "media/logo.svg",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"id": "tilemapExplorer",
"name": "%makecode.tilemapExplorer.title%",
"icon": "media/logo.svg"
"icon": "media/logo.svg",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"id": "songExplorer",
"name": "%makecode.songExplorer.title%",
"icon": "media/logo.svg"
"icon": "media/logo.svg",
"when": "makecode.hasMakeCodeProjectOpen"
}
]
},
Expand All @@ -206,9 +210,9 @@
"menus": {
"editor/context": [
{
"when": "resourceLangId == typescript && makecode.extensionActive",
"when": "resourceLangId == typescript && makecode.extensionActive && makecode.hasMakeCodeProjectOpen",
"command": "makecode.simulate",
"group": "navigation"
"group": "debug"
}
],
"view/title": [
Expand Down Expand Up @@ -283,6 +287,34 @@
{
"command": "makecode.createTilemap",
"when": "false"
},
{
"command": "makecode.build",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"command": "makecode.simulate",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"command": "makecode.clean",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"command": "makecode.shareProject",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"command": "makecode.addDependency",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"command": "makecode.removeDependency",
"when": "makecode.hasMakeCodeProjectOpen"
},
{
"command": "makecode.refreshAssets",
"when": "makecode.hasMakeCodeProjectOpen"
}
]
},
Expand Down
47 changes: 34 additions & 13 deletions src/web/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ export function activate(context: vscode.ExtensionContext) {
context.subscriptions.push(
vscode.window.registerTreeDataProvider("songExplorer", new JResTreeProvider("song"))
);
context.subscriptions.push(
vscode.workspace.onDidChangeWorkspaceFolders(e => {
maybeSetInMakeCodeProjectAsync();
})
)

// This key is not sensitive, and is publicly available in client side apps logging to AI
const appInsightsKey = "0c6ae279ed8443289764825290e4f9e2-1a736e7c-1324-4338-be46-fc2a58ae4d14-7255";
Expand All @@ -124,28 +129,42 @@ export function activate(context: vscode.ExtensionContext) {

// Set a context key to indicate that we have activated, so context menu commands can show
vscode.commands.executeCommand('setContext', 'makecode.extensionActive', true);
maybeSetInMakeCodeProjectAsync();
}

async function chooseWorkspaceAsync(kind: "empty" | "project" | "any", silent = false): Promise<vscode.WorkspaceFolder | undefined> {
async function maybeSetInMakeCodeProjectAsync() {
const hasProjectFolder = await findOpenFolders("project");
if (!!hasProjectFolder.length) {
setInMakeCodeProjectAsync();
}
}

export function setInMakeCodeProjectAsync() {
vscode.commands.executeCommand("setContext", "makecode.hasMakeCodeProjectOpen", true);
}

async function findOpenFolders(kind: "empty" | "project" | "any") {
const folders = [];
let hasWorkspaceOpen = false;

if (vscode.workspace.workspaceFolders) {
hasWorkspaceOpen = !!vscode.workspace.workspaceFolders.length;
for (const folder of vscode.workspace.workspaceFolders) {
if (kind === "any") {
folders.push(folder);
}
else {
const pxtJSONExists = await fileExistsAsync(vscode.Uri.joinPath(folder.uri, "pxt.json"));
for (const folder of (vscode.workspace.workspaceFolders || [])) {
if (kind === "any") {
folders.push(folder);
}
else {
const pxtJSONExists = await fileExistsAsync(vscode.Uri.joinPath(folder.uri, "pxt.json"));

if ((kind === "project" && pxtJSONExists) || (kind === "empty" && !pxtJSONExists)) {
folders.push(folder);
}
if ((kind === "project" && pxtJSONExists) || (kind === "empty" && !pxtJSONExists)) {
folders.push(folder);
}
}
}

return folders;
}

async function chooseWorkspaceAsync(kind: "empty" | "project" | "any", silent = false): Promise<vscode.WorkspaceFolder | undefined> {
const folders = await findOpenFolders(kind);
const hasWorkspaceOpen = !!vscode.workspace.workspaceFolders?.length;

if (folders.length === 0) {
if (!silent) {
Expand Down Expand Up @@ -302,6 +321,7 @@ export async function importUrlCommand(url?: string, useWorkspace?: vscode.Works
}, async progress => {
try {
await downloadSharedProjectAsync(workspace!, toOpen!);
setInMakeCodeProjectAsync()
}
catch (e) {
showError(vscode.l10n.t("Unable to download project"));
Expand Down Expand Up @@ -502,6 +522,7 @@ async function createCommand() {
});
}

setInMakeCodeProjectAsync();
await renameProjectAsync(workspace, projectName);
}

Expand Down
3 changes: 2 additions & 1 deletion src/web/vfs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as path from "path-browserify"
import * as vscode from "vscode"
import { importUrlCommand } from "./extension";
import { importUrlCommand, setInMakeCodeProjectAsync } from "./extension";

export class VFS implements vscode.FileSystemProvider {
private initializedDirs: {[index: string]: boolean} = {}
Expand Down Expand Up @@ -119,6 +119,7 @@ export class VFS implements vscode.FileSystemProvider {
if (!(await this.existsAsync(pxtJSON))) {
await importUrlCommand(shareId, { name: shareId, uri: projectDir, index: 0 })
}
setInMakeCodeProjectAsync();
this.initializedDirs[shareId] = true;
}

Expand Down