fix: escape interpreter, add links to tutorials

marimo-team · Nov 18, 2024 · f54e4a2 · f54e4a2
1 parent fbf7b32
commit f54e4a2
Show file tree

Hide file tree

Showing 11 changed files with 105 additions and 70 deletions.
diff --git 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"
-		}
-	]
-}
+  "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
@@ -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
@@ -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<CommandPickItem>(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<CommandPickItem[]> {
   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
@@ -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<CommandPickItem>(commands);
+  if (result) {
+    result.handler();
+  }
+}
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<boolean> {
+  try {
+    await execFile(executable, ["--help"]);
+    return true;
+  } catch (error) {
+    return false;
+  }
+}
+
 export async function getInterpreter(): Promise<string | undefined> {
   try {
     if (Config.pythonPath) {