From f54e4a2a5ccee7b2013b2bd8d75b5a63eb5f4cf6 Mon Sep 17 00:00:00 2001 From: Myles Scolnick Date: Mon, 18 Nov 2024 13:09:32 -0500 Subject: [PATCH] fix: escape interpreter, add links to tutorials --- .vscode/tasks.json | 30 ++++++++-------- src/commands/__tests__/show-commands.test.ts | 36 +++++--------------- src/commands/show-commands.ts | 27 ++++++++------- src/commands/tutorial-commands.ts | 31 +++++++++++++++++ src/constants.ts | 1 + src/export/export-as.ts | 7 ++-- src/launcher/controller.ts | 2 +- src/services/health.ts | 2 -- src/utils/__tests__/cmd.test.ts | 10 +++--- src/utils/cmd.ts | 2 +- src/utils/exec.ts | 27 +++++++++++++-- 11 files changed, 105 insertions(+), 70 deletions(-) create mode 100644 src/commands/tutorial-commands.ts diff --git a/.vscode/tasks.json b/.vscode/tasks.json index f46384e..f797214 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -1,16 +1,16 @@ { - "version": "2.0.0", - "tasks": [ - { - "type": "npm", - "script": "build", - "group": { - "kind": "build", - "isDefault": true - }, - "problemMatcher": [], - "label": "npm: build", - "detail": "tsup src/extension.ts --external=vscode" - } - ] -} \ No newline at end of file + "version": "2.0.0", + "tasks": [ + { + "type": "npm", + "script": "build", + "group": { + "kind": "build", + "isDefault": true + }, + "problemMatcher": [], + "label": "npm: build", + "detail": "tsup src/extension.ts --external=vscode" + } + ] +} diff --git a/src/commands/__tests__/show-commands.test.ts b/src/commands/__tests__/show-commands.test.ts index 200f174..3eb945b 100644 --- a/src/commands/__tests__/show-commands.test.ts +++ b/src/commands/__tests__/show-commands.test.ts @@ -17,7 +17,7 @@ describe("showCommands", () => { const serverManager = ServerManager.getInstance(Config); it("should show commands for Kernel", async () => { - const commands = await showKernelCommands(mockKernel, serverManager); + const commands = await showKernelCommands(mockKernel); expect(commands.map((c) => c.label)).toMatchInlineSnapshot(` [ "$(split-horizontal) Open outputs in embedded browser", @@ -26,20 +26,13 @@ describe("showCommands", () => { "$(refresh) Restart kernel", "$(export) Export notebook as...", "", - "$(question) View marimo documentation", - "$(comment-discussion) Join Discord community", - "$(settings) Edit settings", - "$(info) Server status: stopped", ] `); }); it("should show commands for non active Controller", async () => { const commands = ( - await showMarimoControllerCommands( - await createMockController(), - serverManager, - ) + await showMarimoControllerCommands(await createMockController()) ).filter((index) => index.if !== false); expect(commands.map((c) => c.label)).toMatchInlineSnapshot(` [ @@ -49,10 +42,6 @@ describe("showCommands", () => { "", "$(export) Export notebook as...", "", - "$(question) View marimo documentation", - "$(comment-discussion) Join Discord community", - "$(settings) Edit settings", - "$(info) Server status: stopped", ] `); }); @@ -61,9 +50,9 @@ describe("showCommands", () => { const controller = await createMockController(); controller.active = true; controller.currentMode = "run"; - const commands = ( - await showMarimoControllerCommands(controller, serverManager) - ).filter((index) => index.if !== false); + const commands = (await showMarimoControllerCommands(controller)).filter( + (index) => index.if !== false, + ); expect(commands.map((c) => c.label)).toMatchInlineSnapshot(` [ "", @@ -75,10 +64,6 @@ describe("showCommands", () => { "$(close) Stop kernel", "$(export) Export notebook as...", "", - "$(question) View marimo documentation", - "$(comment-discussion) Join Discord community", - "$(settings) Edit settings", - "$(info) Server status: stopped", ] `); }); @@ -87,9 +72,9 @@ describe("showCommands", () => { const controller = await createMockController(); controller.active = true; controller.currentMode = "edit"; - const commands = ( - await showMarimoControllerCommands(controller, serverManager) - ).filter((index) => index.if !== false); + const commands = (await showMarimoControllerCommands(controller)).filter( + (index) => index.if !== false, + ); expect(commands.map((c) => c.label)).toMatchInlineSnapshot(` [ "", @@ -101,10 +86,6 @@ describe("showCommands", () => { "$(close) Stop kernel", "$(export) Export notebook as...", "", - "$(question) View marimo documentation", - "$(comment-discussion) Join Discord community", - "$(settings) Edit settings", - "$(info) Server status: stopped", ] `); }); @@ -116,6 +97,7 @@ describe("showCommands", () => { expect(commands.map((c) => c.label)).toMatchInlineSnapshot(` [ "$(question) View marimo documentation", + "$(bookmark) View tutorials", "$(comment-discussion) Join Discord community", "$(settings) Edit settings", "$(info) Server status: stopped", diff --git a/src/commands/show-commands.ts b/src/commands/show-commands.ts index 4b4ee0d..81320cc 100644 --- a/src/commands/show-commands.ts +++ b/src/commands/show-commands.ts @@ -7,6 +7,7 @@ import { window, } from "vscode"; import { + DISCORD_URL, DOCUMENTATION_URL, EXTENSION_DISPLAY_NAME, EXTENSION_PACKAGE, @@ -20,6 +21,7 @@ import { } from "../notebook/extension"; import { Kernel } from "../notebook/kernel"; import type { ServerManager } from "../services/server-manager"; +import { tutorialCommands } from "./tutorial-commands"; interface CommandPickItem extends QuickPickItem { handler: () => void; @@ -39,15 +41,14 @@ export async function showCommands( let commands: CommandPickItem[] = []; if (controller instanceof Kernel) { - commands = await showKernelCommands(controller, serverManager); + commands = await showKernelCommands(controller); } if (controller instanceof MarimoController) { - commands = await showMarimoControllerCommands(controller, serverManager); - } - if (!controller) { - commands = miscCommands(serverManager); + commands = await showMarimoControllerCommands(controller); } + commands.push(...miscCommands(serverManager)); + const filteredCommands = commands.filter((index) => index.if !== false); const result = await window.showQuickPick(filteredCommands); @@ -56,10 +57,7 @@ export async function showCommands( } } -export function showKernelCommands( - kernel: Kernel, - serverManager: ServerManager, -): CommandPickItem[] { +export function showKernelCommands(kernel: Kernel): CommandPickItem[] { return [ { label: "$(split-horizontal) Open outputs in embedded browser", @@ -90,13 +88,11 @@ export function showKernelCommands( }, }, SEPARATOR, - ...miscCommands(serverManager), ]; } export async function showMarimoControllerCommands( controller: MarimoController, - serverManager: ServerManager, ): Promise { return [ // Non-active commands @@ -186,7 +182,6 @@ export async function showMarimoControllerCommands( }, SEPARATOR, - ...miscCommands(serverManager), ]; } @@ -198,10 +193,16 @@ export function miscCommands(serverManager: ServerManager): CommandPickItem[] { env.openExternal(Uri.parse(DOCUMENTATION_URL)); }, }, + { + label: "$(bookmark) View tutorials", + async handler() { + await tutorialCommands(); + }, + }, { label: "$(comment-discussion) Join Discord community", handler() { - env.openExternal(Uri.parse("https://marimo.io/discord")); + env.openExternal(Uri.parse(DISCORD_URL)); }, }, { diff --git a/src/commands/tutorial-commands.ts b/src/commands/tutorial-commands.ts new file mode 100644 index 0000000..d27509f --- /dev/null +++ b/src/commands/tutorial-commands.ts @@ -0,0 +1,31 @@ +import { type QuickPickItem, ThemeIcon, Uri, env, window } from "vscode"; + +interface CommandPickItem extends QuickPickItem { + handler: () => void; +} + +const TUTORIALS = [ + ["Intro", "https://marimo.app/l/c7h6pz", "book"], + ["Dataflow", "https://marimo.app/l/grhuve", "repo-forked"], + ["UI Elements", "https://marimo.app/l/ia3872", "layout"], + ["Markdown", "https://marimo.app/l/pzxdmn", "markdown"], + ["Plotting", "https://marimo.app/l/lxp1jk", "graph"], + ["SQL", "https://marimo.app/l/7n5flc", "database"], + ["Layout", "https://marimo.app/l/14ovyr", "layout-panel-left"], + ["File Format", "https://marimo.app/l/8n55fd", "file"], + ["Coming from Jupyter", "https://marimo.app/l/z0aerp", "code"], +]; + +export async function tutorialCommands() { + const commands: CommandPickItem[] = TUTORIALS.map(([label, url, icon]) => ({ + label, + description: url, + iconPath: new ThemeIcon(icon), + handler: () => env.openExternal(Uri.parse(url)), + })); + + const result = await window.showQuickPick(commands); + if (result) { + result.handler(); + } +} diff --git a/src/constants.ts b/src/constants.ts index 803df1f..eca2257 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,4 +1,5 @@ export const DOCUMENTATION_URL = "https://docs.marimo.io"; +export const DISCORD_URL = "https://marimo.io/discord?ref=vscode"; export const EXTENSION_PACKAGE = { publisher: "marimo-team", diff --git a/src/export/export-as.ts b/src/export/export-as.ts index 852b69e..56908b8 100644 --- a/src/export/export-as.ts +++ b/src/export/export-as.ts @@ -1,11 +1,10 @@ -import { execSync } from "node:child_process"; import { existsSync } from "node:fs"; import path from "node:path"; import { Uri, ViewColumn, window, workspace } from "vscode"; import { Config } from "../config"; import { logger } from "../logger"; import { printError } from "../utils/errors"; -import { execPython } from "../utils/exec"; +import { execPython, hasPythonModule } from "../utils/exec"; export type ExportType = | "ipynb" @@ -105,11 +104,11 @@ export async function exportNotebookAs( } } -function checkRequirements(format: ExportType) { +async function checkRequirements(format: ExportType) { if (format === "ipynb") { // Check that nbformat is installed try { - execSync("nbformat --version", { stdio: "ignore" }); + await hasPythonModule("nbformat"); } catch { throw new Error( "nbformat is not installed. Please install nbformat, e.g. `pip install nbformat`", diff --git a/src/launcher/controller.ts b/src/launcher/controller.ts index b0b70f7..31809ee 100644 --- a/src/launcher/controller.ts +++ b/src/launcher/controller.ts @@ -18,7 +18,7 @@ import { StatusBar } from "../ui/status-bar"; import { MarimoCmdBuilder } from "../utils/cmd"; import { getInterpreter } from "../utils/exec"; import { ping } from "../utils/network"; -import { getFocusedMarimoTextEditor, isMarimoApp } from "../utils/query"; +import { getFocusedMarimoTextEditor } from "../utils/query"; import { asURL } from "../utils/url"; import { type IMarimoTerminal, MarimoTerminal } from "./terminal"; diff --git a/src/services/health.ts b/src/services/health.ts index 80a7f26..584d8cf 100644 --- a/src/services/health.ts +++ b/src/services/health.ts @@ -14,8 +14,6 @@ export class HealthService { version: string; path: string; }> { - const execAsync = promisify(exec); - try { const bytes = await execPython([Config.marimoPath, "--version"]); const stdout = bytes.toString(); diff --git a/src/utils/__tests__/cmd.test.ts b/src/utils/__tests__/cmd.test.ts index 4d214f6..7891a2f 100644 --- a/src/utils/__tests__/cmd.test.ts +++ b/src/utils/__tests__/cmd.test.ts @@ -20,7 +20,7 @@ describe("MarimoCmdBuilder", () => { .tokenPassword("") .build(); expect(cmd).toMatchInlineSnapshot( - `"marimo edit path/to/file --host=localhost --port=2718 --headless --no-token"`, + `"marimo --yes edit path/to/file --host=localhost --port=2718 --headless --no-token"`, ); }); @@ -36,7 +36,7 @@ describe("MarimoCmdBuilder", () => { .tokenPassword("secret") .build(); expect(cmd).toMatchInlineSnapshot( - `"marimo -d edit path/to/file --host=localhost --port=2718 --headless --token-password=secret"`, + `"marimo --yes -d edit path/to/file --host=localhost --port=2718 --headless --token-password=secret"`, ); }); @@ -52,7 +52,7 @@ describe("MarimoCmdBuilder", () => { .tokenPassword("") .build(); expect(cmd).toMatchInlineSnapshot( - `"marimo run path/to/file --host=localhost --port=2718 --headless --no-token"`, + `"marimo --yes run path/to/file --host=localhost --port=2718 --headless --no-token"`, ); }); @@ -69,7 +69,7 @@ describe("MarimoCmdBuilder", () => { .build(); expect(b).toMatchInlineSnapshot( - `"marimo edit "path/to/some file" --host=localhost --port=2718 --headless --no-token"`, + `"marimo --yes edit "path/to/some file" --host=localhost --port=2718 --headless --no-token"`, ); }); @@ -86,7 +86,7 @@ describe("MarimoCmdBuilder", () => { .build(); expect(b).toMatchInlineSnapshot( - `"marimo edit path/to/file --host=0.0.0.0 --port=2718 --headless --no-token"`, + `"marimo --yes edit path/to/file --host=0.0.0.0 --port=2718 --headless --no-token"`, ); }); }); diff --git a/src/utils/cmd.ts b/src/utils/cmd.ts index a892145..5e3ac89 100644 --- a/src/utils/cmd.ts +++ b/src/utils/cmd.ts @@ -1,5 +1,5 @@ export class MarimoCmdBuilder { - private cmd: string[] = ["marimo"]; + private cmd: string[] = ["marimo", "--yes"]; debug(value: boolean) { if (value) { diff --git a/src/utils/exec.ts b/src/utils/exec.ts index c37bf6f..0c256e5 100644 --- a/src/utils/exec.ts +++ b/src/utils/exec.ts @@ -1,4 +1,4 @@ -import { exec, execSync } from "node:child_process"; +import { execFile, execSync } from "node:child_process"; import { workspace } from "vscode"; import { Config } from "../config"; import { logger } from "../logger"; @@ -10,11 +10,34 @@ import { getInterpreterDetails } from "./python"; * We prefix the command with the path to the python executable */ export async function execPython(command: string[]) { - const interpreter = (await getInterpreter()) || "python"; + let interpreter = (await getInterpreter()) || "python"; logger.info(`Using interpreter: ${interpreter}`); + // if interpreter has spaces, wrap in quotes + if (interpreter.includes(" ")) { + interpreter = `"${interpreter}"`; + } return execSync(`${interpreter} -m ${command.join(" ")}`); } +export async function hasPythonModule(module: string) { + let interpreter = (await getInterpreter()) || "python"; + logger.info(`Using interpreter: ${interpreter}`); + // if interpreter has spaces, wrap in quotes + if (interpreter.includes(" ")) { + interpreter = `"${interpreter}"`; + } + return execSync(`${interpreter} -c 'import ${module}'`); +} + +export async function hasExecutable(executable: string): Promise { + try { + await execFile(executable, ["--help"]); + return true; + } catch (error) { + return false; + } +} + export async function getInterpreter(): Promise { try { if (Config.pythonPath) {