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

feat: Create notebook copies #2227

Merged
merged 11 commits into from
Sep 5, 2024
5 changes: 4 additions & 1 deletion frontend/src/components/modal/ImperativeModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -124,6 +126,7 @@ export function useImperativeModal() {
{opts.description}
</AlertDialogDescription>
<Input
spellCheck={opts.spellCheck}
defaultValue={opts.defaultValue}
className="my-4 h-8"
name="prompt"
Expand All @@ -136,7 +139,7 @@ export function useImperativeModal() {
<AlertDialogCancel onClick={closeModal}>
Cancel
</AlertDialogCancel>
<Button type="submit">Ok</Button>
<Button type="submit">{opts.confirmText ?? "Ok"}</Button>
</AlertDialogFooter>
</form>
</AlertDialogContent>
Expand Down
49 changes: 45 additions & 4 deletions frontend/src/components/pages/home-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getRunningNotebooks,
shutdownSession,
openTutorial,
sendCopy,
} from "@/core/network/requests";
import { combineAsyncData, useAsyncData } from "@/hooks/useAsyncData";
import type React from "react";
Expand All @@ -19,6 +20,7 @@ import {
ChevronRightIcon,
ChevronsDownUpIcon,
ClockIcon,
Copy,
DatabaseIcon,
ExternalLinkIcon,
FileIcon,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -441,9 +443,8 @@ const MarimoFileComponent = ({
</div>
<div className="flex flex-col gap-1 items-end">
<div className="flex gap-3 items-center">
<div>
<SessionShutdownButton filePath={file.path} />
</div>
<CopyNotebookButton filePath={file.path} />
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can put this in notebook actions - so it automatically appears in the notebook dropdown and in the command palette. It doesn't need to be its own button.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good! I was just going off the issue author's suggestions, thought it'd be a simple add

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yea I didn't see the second part of the issue mentioning that. I don't know how common it'll be and does complicate the routes. I think we hold off homepage and consider later

<SessionShutdownButton filePath={file.path} />
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this on the homepage - at least for now.

This is also probably why you were getting mismatch sessions. Since the homepage doesn't have an active session

{openNewTab && (
<ExternalLinkIcon
size={20}
Expand Down Expand Up @@ -516,6 +517,46 @@ const SessionShutdownButton: React.FC<{ filePath: string }> = ({
);
};

const CopyNotebookButton: React.FC<{ filePath: string }> = ({ filePath }) => {
const { openPrompt, closeModal } = useImperativeModal();
return (
<Tooltip content="Copy">
<Button
size={"icon"}
variant="outline"
className="opacity-0 group-hover:opacity-100 hover:bg-accent text-primary border-primary hover:border-primary hover:text-primary bg-background hover:bg-[var(--blue-1)]"
onClick={(e) => {
e.stopPropagation();
e.preventDefault();
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({
description: "Notebook has been copied.",
});
},
});
}}
>
<Copy size={14} />
</Button>
</Tooltip>
);
};

const CreateNewNotebook: React.FC = () => {
const sessionId = generateSessionId();
const initializationId = `__new__${sessionId}`;
Expand Down
1 change: 1 addition & 0 deletions frontend/src/core/islands/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export class IslandsPyodideBridge implements RunRequests, EditRequests {
getUsageStats = throwNotImplemented;
sendRename = throwNotImplemented;
sendSave = throwNotImplemented;
sendCopy = throwNotImplemented;
sendRunScratchpad = throwNotImplemented;
sendStdin = throwNotImplemented;
sendInterrupt = throwNotImplemented;
Expand Down
8 changes: 8 additions & 0 deletions frontend/src/core/network/requests-network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down
1 change: 1 addition & 0 deletions frontend/src/core/network/requests-static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export function createStaticRequests(): EditRequests & RunRequests {
sendRunScratchpad: throwNotInEditMode,
sendRename: throwNotInEditMode,
sendSave: throwNotInEditMode,
sendCopy: throwNotInEditMode,
sendInterrupt: throwNotInEditMode,
sendShutdown: throwNotInEditMode,
sendFormat: throwNotInEditMode,
Expand Down
1 change: 1 addition & 0 deletions frontend/src/core/network/requests-toasting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions frontend/src/core/network/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const {
sendRestart,
syncCellIds,
sendSave,
sendCopy,
sendStdin,
sendFormat,
sendInterrupt,
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/core/network/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -95,6 +96,7 @@ export interface RunRequests {
export interface EditRequests {
sendRename: (request: RenameFileRequest) => Promise<null>;
sendSave: (request: SaveNotebookRequest) => Promise<null>;
sendCopy: (request: CopyNotebookRequest) => Promise<null>;
sendStdin: (request: StdinRequest) => Promise<null>;
sendRun: (request: RunRequest) => Promise<null>;
sendRunScratchpad: (request: RunScratchpadRequest) => Promise<null>;
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/core/wasm/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ export class PyodideBridge implements RunRequests, EditRequests {
return null;
};

sendCopy: EditRequests["sendCopy"] = async () => {
// TODO: Implement copy with pyodide!
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

We can throw not implemented. And I think we can hide this action in the notebook dropdown when isWasm is true

};

sendStdin: EditRequests["sendStdin"] = async (request) => {
await this.rpc.proxy.request.bridge({
functionName: "put_input",
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/core/wasm/worker/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import type { PyodideInterface } from "pyodide";
import type {
CodeCompletionRequest,
CopyNotebookRequest,
ExportAsHTMLRequest,
FileCreateRequest,
FileCreateResponse,
Expand Down Expand Up @@ -66,6 +67,7 @@ export interface RawBridge {
read_snippets(): Promise<Snippets>;
format(request: FormatRequest): Promise<FormatResponse>;
save(request: SaveNotebookRequest): Promise<string>;
copy(request: CopyNotebookRequest): Promise<string>;
save_app_config(request: SaveAppConfigurationRequest): Promise<string>;
save_user_config(request: SaveUserConfigurationRequest): Promise<null>;
rename_file(request: string): Promise<string>;
Expand Down
1 change: 1 addition & 0 deletions marimo/_cli/development/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ def _generate_schema() -> dict[str, Any]:
models.RunScratchpadRequest,
models.SaveAppConfigurationRequest,
models.SaveNotebookRequest,
models.CopyNotebookRequest,
models.SaveUserConfigurationRequest,
models.StdinRequest,
models.SuccessResponse,
Expand Down
29 changes: 29 additions & 0 deletions marimo/_server/api/endpoints/files.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from marimo._server.api.utils import parse_request
from marimo._server.models.models import (
BaseResponse,
CopyNotebookRequest,
OpenFileRequest,
ReadCodeResponse,
RenameFileRequest,
Expand Down Expand Up @@ -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(
Expand Down
11 changes: 10 additions & 1 deletion marimo/_server/file_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from __future__ import annotations

import os
import shutil
from typing import Any, Dict, Optional

from marimo import _loggers
Expand All @@ -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()
Expand Down Expand Up @@ -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(
Expand Down
18 changes: 18 additions & 0 deletions marimo/_server/models/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions openapi/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
43 changes: 43 additions & 0 deletions openapi/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -2367,6 +2406,10 @@ export interface components {
names: string[];
persist: boolean;
};
CopyNotebookRequest: {
source: string;
destination: string;
}
SaveUserConfigurationRequest: {
config: components["schemas"]["MarimoConfig"];
};
Expand Down
Loading