From 19e5df76b53905d7973d4e2b89b3a8088931c12b Mon Sep 17 00:00:00 2001 From: Wasim Sandhu Date: Wed, 4 Sep 2024 20:27:24 -0700 Subject: [PATCH 1/9] wip: Create a notebook copy --- .../src/components/modal/ImperativeModal.tsx | 5 +- frontend/src/components/pages/home-page.tsx | 49 +++++++++++++++++-- frontend/src/core/islands/bridge.ts | 1 + frontend/src/core/network/requests-network.ts | 8 +++ frontend/src/core/network/requests-static.ts | 1 + .../src/core/network/requests-toasting.ts | 1 + frontend/src/core/network/requests.ts | 1 + frontend/src/core/network/types.ts | 2 + frontend/src/core/wasm/bridge.ts | 5 ++ frontend/src/core/wasm/worker/types.ts | 2 + marimo/_cli/development/commands.py | 1 + marimo/_server/api/endpoints/files.py | 29 +++++++++++ marimo/_server/file_manager.py | 11 ++++- marimo/_server/models/models.py | 18 +++++++ openapi/api.yaml | 14 ++++++ openapi/src/api.ts | 43 ++++++++++++++++ 16 files changed, 185 insertions(+), 6 deletions(-) diff --git a/frontend/src/components/modal/ImperativeModal.tsx b/frontend/src/components/modal/ImperativeModal.tsx index 4f075a5f656..8919728ed07 100644 --- a/frontend/src/components/modal/ImperativeModal.tsx +++ b/frontend/src/components/modal/ImperativeModal.tsx @@ -99,6 +99,8 @@ export function useImperativeModal() { title: React.ReactNode; description?: React.ReactNode; defaultValue?: string; + spellCheck?: boolean; + confirmText?: string; onConfirm: (value: string) => void; }) => { context.setModal( @@ -124,6 +126,7 @@ export function useImperativeModal() { {opts.description} Cancel - + diff --git a/frontend/src/components/pages/home-page.tsx b/frontend/src/components/pages/home-page.tsx index b4badd8e817..9b0c6cc17e9 100644 --- a/frontend/src/components/pages/home-page.tsx +++ b/frontend/src/components/pages/home-page.tsx @@ -5,6 +5,7 @@ import { getRunningNotebooks, shutdownSession, openTutorial, + sendCopy, } from "@/core/network/requests"; import { combineAsyncData, useAsyncData } from "@/hooks/useAsyncData"; import type React from "react"; @@ -19,6 +20,7 @@ import { ChevronRightIcon, ChevronsDownUpIcon, ClockIcon, + Copy, DatabaseIcon, ExternalLinkIcon, FileIcon, @@ -73,7 +75,7 @@ import { } from "../home/state"; import { Maps } from "@/utils/maps"; import { Input } from "../ui/input"; -import { Paths } from "@/utils/paths"; +import { PathBuilder, Paths } from "@/utils/paths"; import { DropdownMenu, DropdownMenuTrigger, @@ -441,9 +443,8 @@ const MarimoFileComponent = ({
-
- -
+ + {openNewTab && ( = ({ ); }; +const CopyNotebookButton: React.FC<{ filePath: string }> = ({ filePath }) => { + const { openPrompt, closeModal } = useImperativeModal(); + return ( + + + + ); +}; + const CreateNewNotebook: React.FC = () => { const sessionId = generateSessionId(); const initializationId = `__new__${sessionId}`; diff --git a/frontend/src/core/islands/bridge.ts b/frontend/src/core/islands/bridge.ts index d3bfe1f894f..7c970552307 100644 --- a/frontend/src/core/islands/bridge.ts +++ b/frontend/src/core/islands/bridge.ts @@ -125,6 +125,7 @@ export class IslandsPyodideBridge implements RunRequests, EditRequests { getUsageStats = throwNotImplemented; sendRename = throwNotImplemented; sendSave = throwNotImplemented; + sendCopy = throwNotImplemented; sendRunScratchpad = throwNotImplemented; sendStdin = throwNotImplemented; sendInterrupt = throwNotImplemented; diff --git a/frontend/src/core/network/requests-network.ts b/frontend/src/core/network/requests-network.ts index b1cfa3bb9bd..629a7ee32c5 100644 --- a/frontend/src/core/network/requests-network.ts +++ b/frontend/src/core/network/requests-network.ts @@ -40,6 +40,14 @@ export function createNetworkRequests(): EditRequests & RunRequests { }) .then(handleResponseReturnNull); }, + sendCopy: (request) => { + return marimoClient + .POST("/api/kernel/copy", { + body: request, + parseAs: "text", + }) + .then(handleResponseReturnNull); + }, sendFormat: (request) => { return marimoClient .POST("/api/kernel/format", { diff --git a/frontend/src/core/network/requests-static.ts b/frontend/src/core/network/requests-static.ts index 1aaf22b20fa..7728c10d76e 100644 --- a/frontend/src/core/network/requests-static.ts +++ b/frontend/src/core/network/requests-static.ts @@ -37,6 +37,7 @@ export function createStaticRequests(): EditRequests & RunRequests { sendRunScratchpad: throwNotInEditMode, sendRename: throwNotInEditMode, sendSave: throwNotInEditMode, + sendCopy: throwNotInEditMode, sendInterrupt: throwNotInEditMode, sendShutdown: throwNotInEditMode, sendFormat: throwNotInEditMode, diff --git a/frontend/src/core/network/requests-toasting.ts b/frontend/src/core/network/requests-toasting.ts index 035633b4dfc..b9f0540ab74 100644 --- a/frontend/src/core/network/requests-toasting.ts +++ b/frontend/src/core/network/requests-toasting.ts @@ -18,6 +18,7 @@ export function createErrorToastingRequests( sendRunScratchpad: "Failed to run scratchpad", sendRename: "Failed to rename", sendSave: "Failed to save", + sendCopy: "Failed to copy", sendInterrupt: "Failed to interrupt", sendShutdown: "Failed to shutdown", sendFormat: "Failed to format", diff --git a/frontend/src/core/network/requests.ts b/frontend/src/core/network/requests.ts index 06d7bce9dc7..1a0addf04b0 100644 --- a/frontend/src/core/network/requests.ts +++ b/frontend/src/core/network/requests.ts @@ -31,6 +31,7 @@ export const { sendRestart, syncCellIds, sendSave, + sendCopy, sendStdin, sendFormat, sendInterrupt, diff --git a/frontend/src/core/network/types.ts b/frontend/src/core/network/types.ts index 9625c47c86b..d82ef1b4a1b 100644 --- a/frontend/src/core/network/types.ts +++ b/frontend/src/core/network/types.ts @@ -56,6 +56,7 @@ export type RunScratchpadRequest = schemas["RunScratchpadRequest"]; export type SaveAppConfigurationRequest = schemas["SaveAppConfigurationRequest"]; export type SaveNotebookRequest = schemas["SaveNotebookRequest"]; +export type CopyNotebookRequest = schemas["CopyNotebookRequest"]; export type SaveUserConfigurationRequest = schemas["SaveUserConfigurationRequest"]; export interface SetCellConfigRequest { @@ -95,6 +96,7 @@ export interface RunRequests { export interface EditRequests { sendRename: (request: RenameFileRequest) => Promise; sendSave: (request: SaveNotebookRequest) => Promise; + sendCopy: (request: CopyNotebookRequest) => Promise; sendStdin: (request: StdinRequest) => Promise; sendRun: (request: RunRequest) => Promise; sendRunScratchpad: (request: RunScratchpadRequest) => Promise; diff --git a/frontend/src/core/wasm/bridge.ts b/frontend/src/core/wasm/bridge.ts index 6e6cd1ce448..2f55d8da0b4 100644 --- a/frontend/src/core/wasm/bridge.ts +++ b/frontend/src/core/wasm/bridge.ts @@ -187,6 +187,11 @@ export class PyodideBridge implements RunRequests, EditRequests { return null; }; + sendCopy: EditRequests["sendCopy"] = async () => { + // TODO: Implement copy with pyodide! + return null; + }; + sendStdin: EditRequests["sendStdin"] = async (request) => { await this.rpc.proxy.request.bridge({ functionName: "put_input", diff --git a/frontend/src/core/wasm/worker/types.ts b/frontend/src/core/wasm/worker/types.ts index 34054fa0c36..3364ce192d5 100644 --- a/frontend/src/core/wasm/worker/types.ts +++ b/frontend/src/core/wasm/worker/types.ts @@ -2,6 +2,7 @@ import type { PyodideInterface } from "pyodide"; import type { CodeCompletionRequest, + CopyNotebookRequest, ExportAsHTMLRequest, FileCreateRequest, FileCreateResponse, @@ -66,6 +67,7 @@ export interface RawBridge { read_snippets(): Promise; format(request: FormatRequest): Promise; save(request: SaveNotebookRequest): Promise; + copy(request: CopyNotebookRequest): Promise; save_app_config(request: SaveAppConfigurationRequest): Promise; save_user_config(request: SaveUserConfigurationRequest): Promise; rename_file(request: string): Promise; diff --git a/marimo/_cli/development/commands.py b/marimo/_cli/development/commands.py index 9169d61fefc..4e818bc3fd8 100644 --- a/marimo/_cli/development/commands.py +++ b/marimo/_cli/development/commands.py @@ -149,6 +149,7 @@ def _generate_schema() -> dict[str, Any]: models.RunScratchpadRequest, models.SaveAppConfigurationRequest, models.SaveNotebookRequest, + models.CopyNotebookRequest, models.SaveUserConfigurationRequest, models.StdinRequest, models.SuccessResponse, diff --git a/marimo/_server/api/endpoints/files.py b/marimo/_server/api/endpoints/files.py index fc14e526604..5574521e966 100644 --- a/marimo/_server/api/endpoints/files.py +++ b/marimo/_server/api/endpoints/files.py @@ -15,6 +15,7 @@ from marimo._server.api.utils import parse_request from marimo._server.models.models import ( BaseResponse, + CopyNotebookRequest, OpenFileRequest, ReadCodeResponse, RenameFileRequest, @@ -182,6 +183,34 @@ async def save( return PlainTextResponse(content=contents) +@router.post("/copy") +@requires("edit") +async def copy( + *, + request: Request, +) -> PlainTextResponse: + """ + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/CopyNotebookRequest" + responses: + 200: + description: Copy notebook + content: + text/plain: + schema: + type: string + """ + app_state = AppState(request) + body = await parse_request(request, cls=CopyNotebookRequest) + session = app_state.require_current_session() + contents = session.app_file_manager.copy(body) + + return PlainTextResponse(content=contents) + + @router.post("/save_app_config") @requires("edit") async def save_app_config( diff --git a/marimo/_server/file_manager.py b/marimo/_server/file_manager.py index 2e94595859f..1c4d9bbcbab 100644 --- a/marimo/_server/file_manager.py +++ b/marimo/_server/file_manager.py @@ -2,6 +2,7 @@ from __future__ import annotations import os +import shutil from typing import Any, Dict, Optional from marimo import _loggers @@ -15,7 +16,10 @@ save_layout_config, ) from marimo._server.api.status import HTTPException, HTTPStatus -from marimo._server.models.models import SaveNotebookRequest +from marimo._server.models.models import ( + CopyNotebookRequest, + SaveNotebookRequest, +) from marimo._server.utils import canonicalize_filename LOGGER = _loggers.marimo_logger() @@ -261,6 +265,11 @@ def save(self, request: SaveNotebookRequest) -> str: persist=request.persist, ) + def copy(self, request: CopyNotebookRequest) -> str: + source, destination = request.source, request.destination + shutil.copy(source, destination) + return os.path.basename(destination) + def to_code(self) -> str: """Read the contents of the unsaved file.""" contents = codegen.generate_filecontents( diff --git a/marimo/_server/models/models.py b/marimo/_server/models/models.py index 1ff4af47861..bbde1e11a3f 100644 --- a/marimo/_server/models/models.py +++ b/marimo/_server/models/models.py @@ -132,6 +132,24 @@ def __post_init__(self) -> None: ), "Mismatched cell_ids and configs" +@dataclass +class CopyNotebookRequest: + # path to app + source: str + destination: str + + # Validate filenames are valid, and destination path does not already exist + def __post_init__(self) -> None: + assert self.source is not None + assert self.destination is not None + assert os.path.exists( + self.source + ), f"File {self.source} does not exist" + assert not os.path.exists( + self.destination + ), f"File {self.destination} already exists" + + @dataclass class SaveAppConfigurationRequest: # partial app config diff --git a/openapi/api.yaml b/openapi/api.yaml index 1e5a19d544a..f3b644d04f3 100644 --- a/openapi/api.yaml +++ b/openapi/api.yaml @@ -2241,6 +2241,20 @@ paths: schema: type: string description: Save the current app + /api/kernel/copy: + post: + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/CopyNotebookRequest' + responses: + 200: + content: + text/plain: + schema: + type: string + description: Copy notebook /api/kernel/save_app_config: post: requestBody: diff --git a/openapi/src/api.ts b/openapi/src/api.ts index 95c27d89945..8464b1fb117 100644 --- a/openapi/src/api.ts +++ b/openapi/src/api.ts @@ -1233,6 +1233,45 @@ export interface paths { patch?: never; trace?: never; }; + "/api/kernel/copy": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: { + content: { + "application/json": components["schemas"]["CopyNotebookRequest"]; + }; + }; + responses: { + /** @description Save the app as a new file */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "text/plain": string; + }; + }; + }; + }; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/kernel/save_app_config": { parameters: { query?: never; @@ -2367,6 +2406,10 @@ export interface components { names: string[]; persist: boolean; }; + CopyNotebookRequest: { + source: string; + destination: string; + } SaveUserConfigurationRequest: { config: components["schemas"]["MarimoConfig"]; }; From fcbb8afa62e794da7f566a0d48cbfac982ee720a Mon Sep 17 00:00:00 2001 From: Wasim Sandhu Date: Wed, 4 Sep 2024 20:41:14 -0700 Subject: [PATCH 2/9] test: Add unit test --- tests/_server/api/endpoints/test_files.py | 26 +++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/_server/api/endpoints/test_files.py b/tests/_server/api/endpoints/test_files.py index 00cfaef0272..c5c374aecc1 100644 --- a/tests/_server/api/endpoints/test_files.py +++ b/tests/_server/api/endpoints/test_files.py @@ -232,6 +232,32 @@ def test_save_app_config(client: TestClient) -> None: assert 'marimo.App(width="medium"' in file_contents +@with_session(SESSION_ID) +def test_copy_file(client: TestClient) -> None: + filename = get_session_manager(client).file_router.get_unique_file_key() + assert filename + assert os.path.exists(filename) + file_contents = open(filename).read() + assert "import marimo as mo" in file_contents + assert 'marimo.App(width="full"' in file_contents + + filename_copy = f"_{os.path.basename(filename)}" + copied_file = os.path.join(os.path.dirname(filename), filename_copy) + response = client.post( + "/api/kernel/copy", + headers=HEADERS, + json={ + "source": filename, + "destination": copied_file, + }, + ) + assert response.status_code == 200, response.text + assert filename_copy in response.text + file_contents = open(copied_file).read() + assert "import marimo as mo" in file_contents + assert 'marimo.App(width="full"' in file_contents + + @with_websocket_session(SESSION_ID) def test_rename_propagates( client: TestClient, websocket: WebSocketTestSession From 910c297df51fd0d8a1aa0179bc4d89824293584b Mon Sep 17 00:00:00 2001 From: Wasim Sandhu Date: Wed, 4 Sep 2024 22:05:36 -0700 Subject: [PATCH 3/9] fix: Revert home page, create notebook action --- .../editor/actions/useCopyNotebook.tsx | 31 ++++++++++ .../editor/actions/useNotebookActions.tsx | 17 ++++-- .../editor/renderers/slides-layout/types.ts | 2 +- frontend/src/components/pages/home-page.tsx | 61 +++---------------- frontend/src/core/codemirror/copilot/types.ts | 4 +- frontend/src/core/wasm/bridge.ts | 3 +- 6 files changed, 58 insertions(+), 60 deletions(-) create mode 100644 frontend/src/components/editor/actions/useCopyNotebook.tsx diff --git a/frontend/src/components/editor/actions/useCopyNotebook.tsx b/frontend/src/components/editor/actions/useCopyNotebook.tsx new file mode 100644 index 00000000000..5ade87ac532 --- /dev/null +++ b/frontend/src/components/editor/actions/useCopyNotebook.tsx @@ -0,0 +1,31 @@ +/* Copyright 2024 Marimo. All rights reserved. */ +import { useImperativeModal } from "@/components/modal/ImperativeModal"; +import { toast } from "@/components/ui/use-toast"; +import { sendCopy } from "@/core/network/requests"; +import { PathBuilder, Paths } from "@/utils/paths"; + +export function useCopyNotebook(filePath: string) { + const { openPrompt, closeModal } = useImperativeModal(); + + return () => { + const pathBuilder = new PathBuilder("/"); + openPrompt({ + title: "Copy notebook", + description: "Enter a new filename for the notebook copy.", + defaultValue: `_${Paths.basename(filePath)}`, + confirmText: "Copy notebook", + spellCheck: false, + onConfirm: (destination: string) => { + sendCopy({ + source: filePath, + destination: pathBuilder.join(Paths.dirname(filePath), destination), + }); + closeModal(); + toast({ + title: "Notebook copied", + description: "A copy of the notebook has been created.", + }); + }, + }); + }; +} diff --git a/frontend/src/components/editor/actions/useNotebookActions.tsx b/frontend/src/components/editor/actions/useNotebookActions.tsx index 0ab296d0125..099c23b06af 100644 --- a/frontend/src/components/editor/actions/useNotebookActions.tsx +++ b/frontend/src/components/editor/actions/useNotebookActions.tsx @@ -61,6 +61,8 @@ import { LAYOUT_TYPES } from "../renderers/types"; import { displayLayoutName, getLayoutIcon } from "../renderers/layout-select"; import { useLayoutState, useLayoutActions } from "@/core/layout/layout"; import { useTogglePresenting } from "@/core/layout/useTogglePresenting"; +import { useCopyNotebook } from "./useCopyNotebook"; +import { isWasm } from "@/core/wasm/utils"; const NOOP_HANDLER = (event?: Event) => { event?.preventDefault(); @@ -78,6 +80,7 @@ export function useNotebookActions() { const notebook = useNotebook(); const { updateCellConfig, undoDeleteCell } = useCellActions(); const restartKernel = useRestartKernel(); + const copyNotebook = useCopyNotebook(filename!); const setCommandPaletteOpen = useSetAtom(commandPaletteAtom); const setKeyboardShortcutsOpen = useSetAtom(keyboardShortcutsAtom); @@ -193,7 +196,7 @@ export function useNotebookActions() { const md = await exportAsMarkdown({ download: false }); downloadBlob( new Blob([md], { type: "text/plain" }), - Filenames.toMarkdown(document.title), + Filenames.toMarkdown(document.title) ); }, }, @@ -204,7 +207,7 @@ export function useNotebookActions() { const code = await readCode(); downloadBlob( new Blob([code.contents], { type: "text/plain" }), - Filenames.toPY(document.title), + Filenames.toPY(document.title) ); }, }, @@ -274,6 +277,12 @@ export function useNotebookActions() { ], }, + { + icon: , + label: "Create notebook copy", + hidden: !filename || isWasm(), + handle: copyNotebook, + }, { icon: , label: "Copy code to clipboard", @@ -294,7 +303,7 @@ export function useNotebookActions() { handle: async () => { const ids = disabledCells.map((cell) => cell.id); const newConfigs = Objects.fromEntries( - ids.map((cellId) => [cellId, { disabled: false }]), + ids.map((cellId) => [cellId, { disabled: false }]) ); // send to BE await saveCellConfig({ configs: newConfigs }); @@ -311,7 +320,7 @@ export function useNotebookActions() { handle: async () => { const ids = enabledCells.map((cell) => cell.id); const newConfigs = Objects.fromEntries( - ids.map((cellId) => [cellId, { disabled: true }]), + ids.map((cellId) => [cellId, { disabled: true }]) ); // send to BE await saveCellConfig({ configs: newConfigs }); diff --git a/frontend/src/components/editor/renderers/slides-layout/types.ts b/frontend/src/components/editor/renderers/slides-layout/types.ts index 5e5e93ff6ec..2d9c2ec6c85 100644 --- a/frontend/src/components/editor/renderers/slides-layout/types.ts +++ b/frontend/src/components/editor/renderers/slides-layout/types.ts @@ -5,7 +5,7 @@ * The serialized form of a slides layout. * This must be backwards-compatible as it is stored on the user's disk. */ -export type SerializedSlidesLayout = {}; +export interface SerializedSlidesLayout {} export interface SlidesLayout extends SerializedSlidesLayout { // No additional properties for now diff --git a/frontend/src/components/pages/home-page.tsx b/frontend/src/components/pages/home-page.tsx index 9b0c6cc17e9..252459c4dbe 100644 --- a/frontend/src/components/pages/home-page.tsx +++ b/frontend/src/components/pages/home-page.tsx @@ -5,7 +5,6 @@ import { getRunningNotebooks, shutdownSession, openTutorial, - sendCopy, } from "@/core/network/requests"; import { combineAsyncData, useAsyncData } from "@/hooks/useAsyncData"; import type React from "react"; @@ -20,7 +19,6 @@ import { ChevronRightIcon, ChevronsDownUpIcon, ClockIcon, - Copy, DatabaseIcon, ExternalLinkIcon, FileIcon, @@ -75,7 +73,7 @@ import { } from "../home/state"; import { Maps } from "@/utils/maps"; import { Input } from "../ui/input"; -import { PathBuilder, Paths } from "@/utils/paths"; +import { Paths } from "@/utils/paths"; import { DropdownMenu, DropdownMenuTrigger, @@ -98,7 +96,7 @@ const HomePage: React.FC = () => { const recentsResponse = useAsyncData(() => getRecentFiles(), []); const workspaceResponse = useAsyncData( () => getWorkspaceFiles({ includeMarkdown }), - [includeMarkdown], + [includeMarkdown] ); useInterval( @@ -106,7 +104,7 @@ const HomePage: React.FC = () => { setNonce((nonce) => nonce + 1); }, // Refresh every 10 seconds, or when the document becomes visible - { delayMs: 10_000, whenVisible: true }, + { delayMs: 10_000, whenVisible: true } ); const runningResponse = useAsyncData(async () => { @@ -117,7 +115,7 @@ const HomePage: React.FC = () => { const response = combineAsyncData( recentsResponse, workspaceResponse, - runningResponse, + runningResponse ); if (response.error) { @@ -332,7 +330,7 @@ const Node = ({ node, style }: NodeRendererProps) => {
{ evt.stopPropagation(); @@ -443,8 +441,9 @@ const MarimoFileComponent = ({
- - +
+ +
{openNewTab && ( = ({ }) => { const { openConfirm, closeModal } = useImperativeModal(); const { runningNotebooks, setRunningNotebooks } = useContext( - RunningNotebooksContext, + RunningNotebooksContext ); if (!runningNotebooks.has(filePath)) { return null; @@ -495,7 +494,7 @@ const SessionShutdownButton: React.FC<{ filePath: string }> = ({ sessionId: ids.sessionId as SessionId, }).then((response) => { setRunningNotebooks( - Maps.keyBy(response.files, (file) => file.path), + Maps.keyBy(response.files, (file) => file.path) ); }); closeModal(); @@ -517,46 +516,6 @@ const SessionShutdownButton: React.FC<{ filePath: string }> = ({ ); }; -const CopyNotebookButton: React.FC<{ filePath: string }> = ({ filePath }) => { - const { openPrompt, closeModal } = useImperativeModal(); - return ( - - - - ); -}; - const CreateNewNotebook: React.FC = () => { const sessionId = generateSessionId(); const initializationId = `__new__${sessionId}`; diff --git a/frontend/src/core/codemirror/copilot/types.ts b/frontend/src/core/codemirror/copilot/types.ts index 1ecb6303b1a..12bc334ea86 100644 --- a/frontend/src/core/codemirror/copilot/types.ts +++ b/frontend/src/core/codemirror/copilot/types.ts @@ -1,7 +1,7 @@ /* Copyright 2024 Marimo. All rights reserved. */ /* eslint-disable @typescript-eslint/no-empty-interface */ -export type CopilotSignInInitiateParams = {}; +export interface CopilotSignInInitiateParams {} export interface CopilotSignInInitiateResult { verificationUri: string; status: string; @@ -33,7 +33,7 @@ export interface CopilotSignInConfirmResult { status: CopilotStatus; user: string; } -export type CopilotSignOutParams = {}; +export interface CopilotSignOutParams {} export interface CopilotSignOutResult { status: CopilotStatus; } diff --git a/frontend/src/core/wasm/bridge.ts b/frontend/src/core/wasm/bridge.ts index 2f55d8da0b4..01ed06d1f10 100644 --- a/frontend/src/core/wasm/bridge.ts +++ b/frontend/src/core/wasm/bridge.ts @@ -188,8 +188,7 @@ export class PyodideBridge implements RunRequests, EditRequests { }; sendCopy: EditRequests["sendCopy"] = async () => { - // TODO: Implement copy with pyodide! - return null; + throwNotImplemented(); }; sendStdin: EditRequests["sendStdin"] = async (request) => { From 99fd55205aaf4803da15ae2e83ea5b6fdb5377ac Mon Sep 17 00:00:00 2001 From: Wasim Sandhu Date: Wed, 4 Sep 2024 22:14:49 -0700 Subject: [PATCH 4/9] chore: Lint, typecheck, format --- .../components/editor/actions/useCopyNotebook.tsx | 5 ++++- .../components/editor/actions/useNotebookActions.tsx | 10 +++++----- .../editor/renderers/slides-layout/types.ts | 2 +- frontend/src/components/pages/home-page.tsx | 12 ++++++------ frontend/src/core/codemirror/copilot/types.ts | 4 ++-- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/frontend/src/components/editor/actions/useCopyNotebook.tsx b/frontend/src/components/editor/actions/useCopyNotebook.tsx index 5ade87ac532..0c2d2e33bb3 100644 --- a/frontend/src/components/editor/actions/useCopyNotebook.tsx +++ b/frontend/src/components/editor/actions/useCopyNotebook.tsx @@ -4,10 +4,13 @@ import { toast } from "@/components/ui/use-toast"; import { sendCopy } from "@/core/network/requests"; import { PathBuilder, Paths } from "@/utils/paths"; -export function useCopyNotebook(filePath: string) { +export function useCopyNotebook(filePath: string | null) { const { openPrompt, closeModal } = useImperativeModal(); return () => { + if (!filePath) { + return null; + } const pathBuilder = new PathBuilder("/"); openPrompt({ title: "Copy notebook", diff --git a/frontend/src/components/editor/actions/useNotebookActions.tsx b/frontend/src/components/editor/actions/useNotebookActions.tsx index 099c23b06af..facdddc553e 100644 --- a/frontend/src/components/editor/actions/useNotebookActions.tsx +++ b/frontend/src/components/editor/actions/useNotebookActions.tsx @@ -80,7 +80,7 @@ export function useNotebookActions() { const notebook = useNotebook(); const { updateCellConfig, undoDeleteCell } = useCellActions(); const restartKernel = useRestartKernel(); - const copyNotebook = useCopyNotebook(filename!); + const copyNotebook = useCopyNotebook(filename); const setCommandPaletteOpen = useSetAtom(commandPaletteAtom); const setKeyboardShortcutsOpen = useSetAtom(keyboardShortcutsAtom); @@ -196,7 +196,7 @@ export function useNotebookActions() { const md = await exportAsMarkdown({ download: false }); downloadBlob( new Blob([md], { type: "text/plain" }), - Filenames.toMarkdown(document.title) + Filenames.toMarkdown(document.title), ); }, }, @@ -207,7 +207,7 @@ export function useNotebookActions() { const code = await readCode(); downloadBlob( new Blob([code.contents], { type: "text/plain" }), - Filenames.toPY(document.title) + Filenames.toPY(document.title), ); }, }, @@ -303,7 +303,7 @@ export function useNotebookActions() { handle: async () => { const ids = disabledCells.map((cell) => cell.id); const newConfigs = Objects.fromEntries( - ids.map((cellId) => [cellId, { disabled: false }]) + ids.map((cellId) => [cellId, { disabled: false }]), ); // send to BE await saveCellConfig({ configs: newConfigs }); @@ -320,7 +320,7 @@ export function useNotebookActions() { handle: async () => { const ids = enabledCells.map((cell) => cell.id); const newConfigs = Objects.fromEntries( - ids.map((cellId) => [cellId, { disabled: true }]) + ids.map((cellId) => [cellId, { disabled: true }]), ); // send to BE await saveCellConfig({ configs: newConfigs }); diff --git a/frontend/src/components/editor/renderers/slides-layout/types.ts b/frontend/src/components/editor/renderers/slides-layout/types.ts index 2d9c2ec6c85..5e5e93ff6ec 100644 --- a/frontend/src/components/editor/renderers/slides-layout/types.ts +++ b/frontend/src/components/editor/renderers/slides-layout/types.ts @@ -5,7 +5,7 @@ * The serialized form of a slides layout. * This must be backwards-compatible as it is stored on the user's disk. */ -export interface SerializedSlidesLayout {} +export type SerializedSlidesLayout = {}; export interface SlidesLayout extends SerializedSlidesLayout { // No additional properties for now diff --git a/frontend/src/components/pages/home-page.tsx b/frontend/src/components/pages/home-page.tsx index 252459c4dbe..b4badd8e817 100644 --- a/frontend/src/components/pages/home-page.tsx +++ b/frontend/src/components/pages/home-page.tsx @@ -96,7 +96,7 @@ const HomePage: React.FC = () => { const recentsResponse = useAsyncData(() => getRecentFiles(), []); const workspaceResponse = useAsyncData( () => getWorkspaceFiles({ includeMarkdown }), - [includeMarkdown] + [includeMarkdown], ); useInterval( @@ -104,7 +104,7 @@ const HomePage: React.FC = () => { setNonce((nonce) => nonce + 1); }, // Refresh every 10 seconds, or when the document becomes visible - { delayMs: 10_000, whenVisible: true } + { delayMs: 10_000, whenVisible: true }, ); const runningResponse = useAsyncData(async () => { @@ -115,7 +115,7 @@ const HomePage: React.FC = () => { const response = combineAsyncData( recentsResponse, workspaceResponse, - runningResponse + runningResponse, ); if (response.error) { @@ -330,7 +330,7 @@ const Node = ({ node, style }: NodeRendererProps) => {
{ evt.stopPropagation(); @@ -466,7 +466,7 @@ const SessionShutdownButton: React.FC<{ filePath: string }> = ({ }) => { const { openConfirm, closeModal } = useImperativeModal(); const { runningNotebooks, setRunningNotebooks } = useContext( - RunningNotebooksContext + RunningNotebooksContext, ); if (!runningNotebooks.has(filePath)) { return null; @@ -494,7 +494,7 @@ const SessionShutdownButton: React.FC<{ filePath: string }> = ({ sessionId: ids.sessionId as SessionId, }).then((response) => { setRunningNotebooks( - Maps.keyBy(response.files, (file) => file.path) + Maps.keyBy(response.files, (file) => file.path), ); }); closeModal(); diff --git a/frontend/src/core/codemirror/copilot/types.ts b/frontend/src/core/codemirror/copilot/types.ts index 12bc334ea86..1ecb6303b1a 100644 --- a/frontend/src/core/codemirror/copilot/types.ts +++ b/frontend/src/core/codemirror/copilot/types.ts @@ -1,7 +1,7 @@ /* Copyright 2024 Marimo. All rights reserved. */ /* eslint-disable @typescript-eslint/no-empty-interface */ -export interface CopilotSignInInitiateParams {} +export type CopilotSignInInitiateParams = {}; export interface CopilotSignInInitiateResult { verificationUri: string; status: string; @@ -33,7 +33,7 @@ export interface CopilotSignInConfirmResult { status: CopilotStatus; user: string; } -export interface CopilotSignOutParams {} +export type CopilotSignOutParams = {}; export interface CopilotSignOutResult { status: CopilotStatus; } From 13a8de3b2838e74cb72a2111124d8f5c90eda161 Mon Sep 17 00:00:00 2001 From: Wasim Sandhu Date: Wed, 4 Sep 2024 22:45:07 -0700 Subject: [PATCH 5/9] fix: Open new notebook copy after create --- .../editor/actions/useCopyNotebook.tsx | 17 ++++++++++++----- .../editor/actions/useNotebookActions.tsx | 3 ++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/frontend/src/components/editor/actions/useCopyNotebook.tsx b/frontend/src/components/editor/actions/useCopyNotebook.tsx index 0c2d2e33bb3..3e37c06d211 100644 --- a/frontend/src/components/editor/actions/useCopyNotebook.tsx +++ b/frontend/src/components/editor/actions/useCopyNotebook.tsx @@ -4,30 +4,37 @@ import { toast } from "@/components/ui/use-toast"; import { sendCopy } from "@/core/network/requests"; import { PathBuilder, Paths } from "@/utils/paths"; -export function useCopyNotebook(filePath: string | null) { +export function useCopyNotebook(source: string | null) { const { openPrompt, closeModal } = useImperativeModal(); return () => { - if (!filePath) { + if (!source) { return null; } const pathBuilder = new PathBuilder("/"); + const filename = Paths.basename(source); + openPrompt({ title: "Copy notebook", description: "Enter a new filename for the notebook copy.", - defaultValue: `_${Paths.basename(filePath)}`, + defaultValue: `_${filename}`, confirmText: "Copy notebook", spellCheck: false, onConfirm: (destination: string) => { sendCopy({ - source: filePath, - destination: pathBuilder.join(Paths.dirname(filePath), destination), + source: source, + destination: pathBuilder.join(Paths.dirname(source), destination), }); closeModal(); toast({ title: "Notebook copied", description: "A copy of the notebook has been created.", }); + const notebookCopy = window.location.href.replace( + filename, + destination, + ); + window.open(notebookCopy); }, }); }; diff --git a/frontend/src/components/editor/actions/useNotebookActions.tsx b/frontend/src/components/editor/actions/useNotebookActions.tsx index facdddc553e..b22ea547863 100644 --- a/frontend/src/components/editor/actions/useNotebookActions.tsx +++ b/frontend/src/components/editor/actions/useNotebookActions.tsx @@ -29,6 +29,7 @@ import { PresentationIcon, EditIcon, LayoutTemplateIcon, + Files, } from "lucide-react"; import { commandPaletteAtom } from "../controls/command-palette"; import { useCellActions, useNotebook } from "@/core/cells/cells"; @@ -278,7 +279,7 @@ export function useNotebookActions() { }, { - icon: , + icon: , label: "Create notebook copy", hidden: !filename || isWasm(), handle: copyNotebook, From 0330f87b35efde0c439120e97333cd188f4636a9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 5 Sep 2024 05:48:00 +0000 Subject: [PATCH 6/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- openapi/src/api.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi/src/api.ts b/openapi/src/api.ts index 8464b1fb117..3bd54c5777c 100644 --- a/openapi/src/api.ts +++ b/openapi/src/api.ts @@ -2409,7 +2409,7 @@ export interface components { CopyNotebookRequest: { source: string; destination: string; - } + }; SaveUserConfigurationRequest: { config: components["schemas"]["MarimoConfig"]; }; From 5a33dbe03fba83e7037a7fc611a226bfda80ee34 Mon Sep 17 00:00:00 2001 From: Wasim Sandhu Date: Wed, 4 Sep 2024 23:06:14 -0700 Subject: [PATCH 7/9] fix: Error handling --- .../editor/actions/useCopyNotebook.tsx | 31 ++++++++++++------- marimo/_server/models/models.py | 5 +-- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/frontend/src/components/editor/actions/useCopyNotebook.tsx b/frontend/src/components/editor/actions/useCopyNotebook.tsx index 3e37c06d211..304db6b907c 100644 --- a/frontend/src/components/editor/actions/useCopyNotebook.tsx +++ b/frontend/src/components/editor/actions/useCopyNotebook.tsx @@ -24,17 +24,26 @@ export function useCopyNotebook(source: string | null) { sendCopy({ source: source, destination: pathBuilder.join(Paths.dirname(source), destination), - }); - closeModal(); - toast({ - title: "Notebook copied", - description: "A copy of the notebook has been created.", - }); - const notebookCopy = window.location.href.replace( - filename, - destination, - ); - window.open(notebookCopy); + }) + .then(() => { + closeModal(); + toast({ + title: "Notebook copied", + description: "A copy of the notebook has been created.", + }); + const notebookCopy = window.location.href.replace( + filename, + destination, + ); + window.open(notebookCopy); + }) + .catch((error) => { + toast({ + title: "Failed to copy notebook", + description: error.detail, + variant: "danger", + }); + }); }, }); }; diff --git a/marimo/_server/models/models.py b/marimo/_server/models/models.py index bbde1e11a3f..da0f3ebdb72 100644 --- a/marimo/_server/models/models.py +++ b/marimo/_server/models/models.py @@ -140,14 +140,15 @@ class CopyNotebookRequest: # Validate filenames are valid, and destination path does not already exist def __post_init__(self) -> None: + destination = os.path.basename(self.destination) assert self.source is not None assert self.destination is not None assert os.path.exists( self.source - ), f"File {self.source} does not exist" + ), f'File "{self.source}" does not exist. Please save the notebook and try again.' assert not os.path.exists( self.destination - ), f"File {self.destination} already exists" + ), f'File "{destination}" already exists in this directory.' @dataclass From 2df9c99002f26237bad0acb738d178d557f0a9e1 Mon Sep 17 00:00:00 2001 From: Wasim Sandhu Date: Wed, 4 Sep 2024 23:09:01 -0700 Subject: [PATCH 8/9] fix: Typecheck --- marimo/_server/models/models.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/marimo/_server/models/models.py b/marimo/_server/models/models.py index da0f3ebdb72..12d212dc7df 100644 --- a/marimo/_server/models/models.py +++ b/marimo/_server/models/models.py @@ -143,9 +143,10 @@ def __post_init__(self) -> None: destination = os.path.basename(self.destination) assert self.source is not None assert self.destination is not None - assert os.path.exists( - self.source - ), f'File "{self.source}" does not exist. Please save the notebook and try again.' + assert os.path.exists(self.source), ( + f'File "{self.source}" does not exist.' + + "Please save the notebook and try again." + ) assert not os.path.exists( self.destination ), f'File "{destination}" already exists in this directory.' From fa122710a5f9cdcff46a361cf0a13155fabd982e Mon Sep 17 00:00:00 2001 From: Wasim Sandhu Date: Thu, 5 Sep 2024 10:05:41 -0700 Subject: [PATCH 9/9] fix: Make change requests --- .../editor/actions/useCopyNotebook.tsx | 28 ++++++------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/frontend/src/components/editor/actions/useCopyNotebook.tsx b/frontend/src/components/editor/actions/useCopyNotebook.tsx index 304db6b907c..949f37b4f11 100644 --- a/frontend/src/components/editor/actions/useCopyNotebook.tsx +++ b/frontend/src/components/editor/actions/useCopyNotebook.tsx @@ -11,7 +11,7 @@ export function useCopyNotebook(source: string | null) { if (!source) { return null; } - const pathBuilder = new PathBuilder("/"); + const pathBuilder = PathBuilder.guessDeliminator(source); const filename = Paths.basename(source); openPrompt({ @@ -24,26 +24,14 @@ export function useCopyNotebook(source: string | null) { sendCopy({ source: source, destination: pathBuilder.join(Paths.dirname(source), destination), - }) - .then(() => { - closeModal(); - toast({ - title: "Notebook copied", - description: "A copy of the notebook has been created.", - }); - const notebookCopy = window.location.href.replace( - filename, - destination, - ); - window.open(notebookCopy); - }) - .catch((error) => { - toast({ - title: "Failed to copy notebook", - description: error.detail, - variant: "danger", - }); + }).then(() => { + closeModal(); + toast({ + title: "Notebook copied", + description: "A copy of the notebook has been created.", }); + window.open(`/?file=${destination}`, "_blank"); + }); }, }); };