From a071cc1414425cc967ee2d047b73b80acff54878 Mon Sep 17 00:00:00 2001 From: Tim Mok Date: Fri, 5 Apr 2024 15:57:00 -0400 Subject: [PATCH 1/9] Support multiple formats for plot saving Add SVG support --- .../positron/positron_ipykernel/plot_comm.py | 4 +++ .../positron/positron_ipykernel/plots.py | 17 +++++----- positron/comms/plot-backend-openrpc.json | 7 +++++ .../modalDialogs/savePlotModalDialog.tsx | 31 ++++++++++++------- .../common/languageRuntimePlotClient.ts | 20 ++++++++---- .../common/positronPlotComm.ts | 5 +-- 6 files changed, 58 insertions(+), 26 deletions(-) diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/plot_comm.py b/extensions/positron-python/python_files/positron/positron_ipykernel/plot_comm.py index fdeaa8dcd67..57a160b890e 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/plot_comm.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/plot_comm.py @@ -59,6 +59,10 @@ class RenderParams(BaseModel): description="The pixel ratio of the display device", ) + format: str = Field( + description="The requested plot format", + ) + class RenderRequest(BaseModel): """ diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py index 9ecae76f816..6cc9f054a1f 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py @@ -112,12 +112,14 @@ def handle_msg(self, msg: CommMessage[PlotBackendMessageContent], raw_msg: JsonR width_px = request.params.width or 0 height_px = request.params.height or 0 pixel_ratio = request.params.pixel_ratio or 1.0 + format = request.params.format or "png" if width_px != 0 and height_px != 0: - format_dict = self._resize_pickled_figure(pickled, width_px, height_px, pixel_ratio) - data = format_dict["image/png"] - output = PlotResult(data=data, mime_type="image/png").dict() - figure_comm.send_result(data=output, metadata={"mime_type": "image/png"}) + format_dict = self._resize_pickled_figure(pickled, width_px, height_px, pixel_ratio, [format]) + mime_type = f'image/{format}' + data = format_dict[mime_type] + output = PlotResult(data=data, mime_type=mime_type).dict() + figure_comm.send_result(data=output, metadata={"mime_type": mime_type}) else: logger.warning(f"Unhandled request: {request}") @@ -170,7 +172,7 @@ def _resize_pickled_figure( new_width_px: int = 614, new_height_px: int = 460, pixel_ratio: float = 1.0, - formats: list = ["image/png"], + formats: list = ["png"], ) -> dict: # Delay importing matplotlib until the kernel and shell has been # initialized otherwise the graphics backend will be reset to the gui @@ -215,11 +217,12 @@ def _resize_pickled_figure( # Render the figure to a buffer # using format_display_data() crops the figure to smaller than requested size - figure.savefig(figure_buffer, format="png") + figure.savefig(figure_buffer, format=formats[0]) figure_buffer.seek(0) image_data = base64.b64encode(figure_buffer.read()).decode() + key = f'image/{formats[0]}' - format_dict = {"image/png": image_data} + format_dict = {key: image_data} plt.close(figure) diff --git a/positron/comms/plot-backend-openrpc.json b/positron/comms/plot-backend-openrpc.json index ae607cc8cc9..c022dcfb79b 100644 --- a/positron/comms/plot-backend-openrpc.json +++ b/positron/comms/plot-backend-openrpc.json @@ -30,6 +30,13 @@ "schema": { "type": "number" } + }, + { + "name": "format", + "description": "The requested plot format", + "schema": { + "type": "string" + } } ], "result": { diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx index 1903bcdfdb2..31c9f7d386e 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx @@ -18,12 +18,18 @@ import { OKCancelActionBar } from 'vs/workbench/browser/positronComponents/posit import { PositronModalDialog } from 'vs/workbench/browser/positronComponents/positronModalDialog/positronModalDialog'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { PositronModalReactRenderer } from 'vs/workbench/browser/positronModalReactRenderer/positronModalReactRenderer'; +import { FileFilter } from 'electron'; export interface SavePlotOptions { uri: string; path: URI; } +export enum PlotFormat { + PNG = 'PNG', + SVG = 'SVG', +} + const SAVE_PLOT_MODAL_DIALOG_WIDTH = 500; const SAVE_PLOT_MODAL_DIALOG_HEIGHT = 600; const BASE_DPI = 100; // matplotlib default DPI @@ -58,6 +64,7 @@ export const showSavePlotModalDialog = ( renderer.render( { const [rendering, setRendering] = React.useState(false); const inputRef = React.useRef(null); + const filterEntries: FileFilter[] = []; + for (const filter in PlotFormat) { + filterEntries.push({ extensions: [filter.toLowerCase()], name: filter }); + } + React.useEffect(() => { setUri(props.plotClient.lastRender?.uri ?? ''); }, [props.plotClient.lastRender?.uri]); @@ -124,13 +137,7 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { const browseHandler = async () => { const uri = await props.fileDialogService.showSaveDialog({ title: localize('positron.savePlotModalDialog.title', "Save Plot"), - filters: - [ - { - extensions: ['png'], - name: 'PNG', - }, - ], + filters: filterEntries, }); if (uri?.fsPath.length) { @@ -141,7 +148,9 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { const acceptHandler = async () => { if (validateInput()) { setRendering(true); - const plotResult = await generatePreview(); + const extension = path.value.fsPath.split('.').pop()?.toLowerCase(); + const format = extension === 'png' || extension === 'svg' ? extension : 'png'; + const plotResult = await generatePreview(format); if (plotResult) { props.savePlotCallback({ uri: plotResult.uri, path: path.value }); @@ -162,15 +171,15 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { } setRendering(true); try { - const plotResult = await generatePreview(); + const plotResult = await generatePreview('png'); setUri(plotResult.uri); } finally { setRendering(false); } }; - const generatePreview = async (): Promise => { - return props.plotClient.preview(height.value, width.value, dpi.value / BASE_DPI); + const generatePreview = async (format: 'png' | 'svg'): Promise => { + return props.plotClient.preview(height.value, width.value, dpi.value / BASE_DPI, format); }; const previewButton = () => { diff --git a/src/vs/workbench/services/languageRuntime/common/languageRuntimePlotClient.ts b/src/vs/workbench/services/languageRuntime/common/languageRuntimePlotClient.ts index 97baccfb878..5b19883716f 100644 --- a/src/vs/workbench/services/languageRuntime/common/languageRuntimePlotClient.ts +++ b/src/vs/workbench/services/languageRuntime/common/languageRuntimePlotClient.ts @@ -76,6 +76,9 @@ interface RenderRequest { /** The pixel ratio of the device for which the plot was rendered */ pixel_ratio: number; + + /** The format of the plot */ + format: string; } /** @@ -246,9 +249,10 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien * @param height The plot height, in pixels * @param width The plot width, in pixels * @param pixel_ratio The device pixel ratio (e.g. 1 for standard displays, 2 for retina displays) + * @param format The format of the plot ('png', 'svg') * @returns A promise that resolves to a rendered image, or rejects with an error. */ - public render(height: number, width: number, pixel_ratio: number): Promise { + public render(height: number, width: number, pixel_ratio: number, format = 'png'): Promise { // Deal with whole pixels only height = Math.floor(height); width = Math.floor(width); @@ -269,7 +273,8 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien const request: RenderRequest = { height, width, - pixel_ratio + pixel_ratio, + format }; const deferred = new DeferredRender(request); @@ -309,7 +314,7 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien * @param pixel_ratio The device pixel ratio (e.g. 1 for standard displays, 2 for retina displays) * @returns A promise that resolves when the render request is scheduled, or rejects with an error. */ - public preview(height: number, width: number, pixel_ratio: number): Promise { + public preview(height: number, width: number, pixel_ratio: number, format: string): Promise { // Deal with whole pixels only height = Math.floor(height); width = Math.floor(width); @@ -318,7 +323,8 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien const request: RenderRequest = { height, width, - pixel_ratio + pixel_ratio, + format }; const deferred = new DeferredRender(request); @@ -384,7 +390,8 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien const renderRequest = request.renderRequest; this._comm.render(renderRequest.height, renderRequest.width, - renderRequest.pixel_ratio).then((response) => { + renderRequest.pixel_ratio, + renderRequest.format).then((response) => { // Ignore if the request was cancelled or already fulfilled if (!request.isComplete) { @@ -490,7 +497,8 @@ export class PlotClientInstance extends Disposable implements IPositronPlotClien const req = new DeferredRender({ height: Math.floor(height!), width: Math.floor(width!), - pixel_ratio: pixel_ratio! + pixel_ratio: pixel_ratio!, + format: this._currentRender?.renderRequest.format ?? 'png' }); this.scheduleRender(req, 0); diff --git a/src/vs/workbench/services/languageRuntime/common/positronPlotComm.ts b/src/vs/workbench/services/languageRuntime/common/positronPlotComm.ts index e263c113b06..95836a370f3 100644 --- a/src/vs/workbench/services/languageRuntime/common/positronPlotComm.ts +++ b/src/vs/workbench/services/languageRuntime/common/positronPlotComm.ts @@ -51,11 +51,12 @@ export class PositronPlotComm extends PositronBaseComm { * @param height The requested plot height, in pixels * @param width The requested plot width, in pixels * @param pixelRatio The pixel ratio of the display device + * @param format The requested plot format * * @returns A rendered plot */ - render(height: number, width: number, pixelRatio: number): Promise { - return super.performRpc('render', ['height', 'width', 'pixel_ratio'], [height, width, pixelRatio]); + render(height: number, width: number, pixelRatio: number, format: string): Promise { + return super.performRpc('render', ['height', 'width', 'pixel_ratio', 'format'], [height, width, pixelRatio, format]); } From ed253d0cb29f56ecb65306672c6b9ec362a43b50 Mon Sep 17 00:00:00 2001 From: Tim Mok Date: Fri, 5 Apr 2024 16:54:41 -0400 Subject: [PATCH 2/9] PDF support --- .../positron/positron_ipykernel/plots.py | 9 +++++-- .../modalDialogs/savePlotModalDialog.tsx | 26 ++++++++++++++----- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py index 6cc9f054a1f..b00568282e7 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py @@ -26,6 +26,11 @@ DEFAULT_HEIGHT_IN = 4.8 BASE_DPI = 100 +MIME_TYPE = { + "png": "image/png", + "svg": "image/svg+xml", + "pdf": "application/pdf", +} class PositronDisplayPublisherHook: def __init__(self, target_name: str, session_mode: SessionMode): @@ -116,7 +121,7 @@ def handle_msg(self, msg: CommMessage[PlotBackendMessageContent], raw_msg: JsonR if width_px != 0 and height_px != 0: format_dict = self._resize_pickled_figure(pickled, width_px, height_px, pixel_ratio, [format]) - mime_type = f'image/{format}' + mime_type = MIME_TYPE[format] data = format_dict[mime_type] output = PlotResult(data=data, mime_type=mime_type).dict() figure_comm.send_result(data=output, metadata={"mime_type": mime_type}) @@ -220,7 +225,7 @@ def _resize_pickled_figure( figure.savefig(figure_buffer, format=formats[0]) figure_buffer.seek(0) image_data = base64.b64encode(figure_buffer.read()).decode() - key = f'image/{formats[0]}' + key = MIME_TYPE[formats[0]] format_dict = {key: image_data} diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx index 31c9f7d386e..8b4e120b098 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx @@ -26,8 +26,9 @@ export interface SavePlotOptions { } export enum PlotFormat { - PNG = 'PNG', - SVG = 'SVG', + PNG = 'png', + SVG = 'svg', + PDF = 'pdf', } const SAVE_PLOT_MODAL_DIALOG_WIDTH = 500; @@ -99,7 +100,7 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { const filterEntries: FileFilter[] = []; for (const filter in PlotFormat) { - filterEntries.push({ extensions: [filter.toLowerCase()], name: filter }); + filterEntries.push({ extensions: [filter.toLowerCase()], name: filter.toUpperCase() }); } React.useEffect(() => { @@ -149,7 +150,20 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { if (validateInput()) { setRendering(true); const extension = path.value.fsPath.split('.').pop()?.toLowerCase(); - const format = extension === 'png' || extension === 'svg' ? extension : 'png'; + // const format = extension === 'png' || extension === 'svg' ? extension : 'png'; + let format = PlotFormat.PNG; + switch (extension) { + case 'svg': + format = PlotFormat.SVG; + break; + case 'pdf': + format = PlotFormat.PDF; + break; + case 'png': + default: + format = PlotFormat.PNG; + break; + } const plotResult = await generatePreview(format); if (plotResult) { @@ -171,14 +185,14 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { } setRendering(true); try { - const plotResult = await generatePreview('png'); + const plotResult = await generatePreview(PlotFormat.PNG); setUri(plotResult.uri); } finally { setRendering(false); } }; - const generatePreview = async (format: 'png' | 'svg'): Promise => { + const generatePreview = async (format: PlotFormat): Promise => { return props.plotClient.preview(height.value, width.value, dpi.value / BASE_DPI, format); }; From a9e4f76da12183ac7d3608cf8ca5f92d83957cc0 Mon Sep 17 00:00:00 2001 From: Tim Mok Date: Mon, 8 Apr 2024 11:50:19 -0400 Subject: [PATCH 3/9] JPEG support --- .../python_files/positron/positron_ipykernel/plots.py | 1 + .../positronPlots/browser/modalDialogs/savePlotModalDialog.tsx | 1 + 2 files changed, 2 insertions(+) diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py index b00568282e7..d2c9e53afbf 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py @@ -30,6 +30,7 @@ "png": "image/png", "svg": "image/svg+xml", "pdf": "application/pdf", + "jpeg": "image/jpeg", } class PositronDisplayPublisherHook: diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx index 8b4e120b098..351f93de67c 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx @@ -29,6 +29,7 @@ export enum PlotFormat { PNG = 'png', SVG = 'svg', PDF = 'pdf', + JPEG = 'jpeg', } const SAVE_PLOT_MODAL_DIALOG_WIDTH = 500; From 58da1839ed6611dc01873481d227f535593f81d7 Mon Sep 17 00:00:00 2001 From: Tim Mok Date: Mon, 8 Apr 2024 15:02:53 -0400 Subject: [PATCH 4/9] simplify format check --- .../modalDialogs/savePlotModalDialog.tsx | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx index 351f93de67c..aa7587106c2 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx @@ -150,21 +150,15 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { const acceptHandler = async () => { if (validateInput()) { setRendering(true); + const extension = path.value.fsPath.split('.').pop()?.toLowerCase(); - // const format = extension === 'png' || extension === 'svg' ? extension : 'png'; - let format = PlotFormat.PNG; - switch (extension) { - case 'svg': - format = PlotFormat.SVG; - break; - case 'pdf': - format = PlotFormat.PDF; - break; - case 'png': - default: - format = PlotFormat.PNG; - break; + let format = extension as PlotFormat; + + // default to PNG if the format is unknown + if (!Object.values(PlotFormat).includes(format)) { + format = PlotFormat.PNG; } + const plotResult = await generatePreview(format); if (plotResult) { From a0e6813c31ff8f91b22976f90c526b2e3bc76689 Mon Sep 17 00:00:00 2001 From: Tim Mok Date: Wed, 10 Apr 2024 13:57:16 -0400 Subject: [PATCH 5/9] cleanup --- positron/comms/plot-backend-openrpc.json | 3 ++- .../positronPlots/browser/modalDialogs/savePlotModalDialog.tsx | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/positron/comms/plot-backend-openrpc.json b/positron/comms/plot-backend-openrpc.json index c022dcfb79b..5a80970a01e 100644 --- a/positron/comms/plot-backend-openrpc.json +++ b/positron/comms/plot-backend-openrpc.json @@ -36,7 +36,8 @@ "description": "The requested plot format", "schema": { "type": "string" - } + }, + "required": false } ], "result": { diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx index aa7587106c2..8d8d5e929e7 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx @@ -66,7 +66,6 @@ export const showSavePlotModalDialog = ( renderer.render( Date: Wed, 10 Apr 2024 16:01:22 -0400 Subject: [PATCH 6/9] Formatting --- .../python_files/positron/positron_ipykernel/plots.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py index d2c9e53afbf..03b061db47f 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/plots.py @@ -33,6 +33,7 @@ "jpeg": "image/jpeg", } + class PositronDisplayPublisherHook: def __init__(self, target_name: str, session_mode: SessionMode): self.target_name = target_name @@ -121,7 +122,9 @@ def handle_msg(self, msg: CommMessage[PlotBackendMessageContent], raw_msg: JsonR format = request.params.format or "png" if width_px != 0 and height_px != 0: - format_dict = self._resize_pickled_figure(pickled, width_px, height_px, pixel_ratio, [format]) + format_dict = self._resize_pickled_figure( + pickled, width_px, height_px, pixel_ratio, [format] + ) mime_type = MIME_TYPE[format] data = format_dict[mime_type] output = PlotResult(data=data, mime_type=mime_type).dict() From e287685a9285ae17107bff03927d3d2bc7a605d9 Mon Sep 17 00:00:00 2001 From: Tim Mok Date: Wed, 10 Apr 2024 16:18:33 -0400 Subject: [PATCH 7/9] Fix tests --- .../positron/positron_ipykernel/tests/test_plots.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_plots.py b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_plots.py index 8f978066790..6462d49cdb5 100644 --- a/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_plots.py +++ b/extensions/positron-python/python_files/positron/positron_ipykernel/tests/test_plots.py @@ -162,7 +162,7 @@ def test_hook_call(hook: PositronDisplayPublisherHook, images_path: Path) -> Non def render_request(comm_id: str, width_px: int = 500, height_px: int = 500, pixel_ratio: int = 1): return json_rpc_request( "render", - {"width": width_px, "height": height_px, "pixel_ratio": pixel_ratio}, + {"width": width_px, "height": height_px, "pixel_ratio": pixel_ratio, "format": "png"}, comm_id=comm_id, ) From 3cab731bd148710e848e7c364c68817ba975c46f Mon Sep 17 00:00:00 2001 From: Tim Mok Date: Fri, 12 Apr 2024 17:13:35 -0400 Subject: [PATCH 8/9] Refactor format saving Add format selector in dialog Browse now sets directory save location Add field for file name Validate path at save time Show error if saving failed --- .../modalDialogs/savePlotModalDialog.css | 20 ++- .../modalDialogs/savePlotModalDialog.tsx | 134 +++++++++++++----- .../browser/positronPlotsService.ts | 14 +- 3 files changed, 127 insertions(+), 41 deletions(-) diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.css b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.css index f0c7fabe429..e6314346abc 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.css +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.css @@ -4,9 +4,10 @@ .plot-preview-input { height: 100%; - grid-template-rows: 1fr 2fr 4px 9fr; + grid-template-rows: 1fr 1fr 2fr 4px 9fr; grid-template-areas: "browse" + "file" "plot-input" "preview-progress" "preview"; @@ -28,6 +29,17 @@ grid-area: input; } +.plot-preview-input .file { + display: flex; + flex-direction: row; + column-gap: 10px; + align-items: flex-end; +} + +.plot-preview-input .file button { + margin-top: 4px; +} + .plot-input div.error { padding-top: 4px; grid-column: 1 / span 3; @@ -40,10 +52,14 @@ width: auto; } -.plot-preview-input .labeled-text-input input { +.plot-preview-input .plot-input .labeled-text-input input { width: 100px; } +.plot-preview-input .file .labeled-text-input input { + width: 200px; +} + .plot-preview-input .preview-progress { grid-area: preview-progress; display: flex; diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx index 8d8d5e929e7..b607fac3114 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx @@ -7,7 +7,7 @@ import * as React from 'react'; import { localize } from 'vs/nls'; import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; import { IRenderedPlot, PlotClientInstance } from 'vs/workbench/services/languageRuntime/common/languageRuntimePlotClient'; -import { IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; +import { IDialogService, IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; import { URI } from 'vs/base/common/uri'; import { ProgressBar } from 'vs/base/browser/ui/positronComponents/progressBar'; import { LabeledTextInput } from 'vs/workbench/browser/positronComponents/positronModalDialog/components/labeledTextInput'; @@ -19,6 +19,9 @@ import { PositronModalDialog } from 'vs/workbench/browser/positronComponents/pos import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { PositronModalReactRenderer } from 'vs/workbench/browser/positronModalReactRenderer/positronModalReactRenderer'; import { FileFilter } from 'electron'; +import { DropDownListBox } from 'vs/workbench/browser/positronComponents/dropDownListBox/dropDownListBox'; +import { DropDownListBoxItem } from 'vs/workbench/browser/positronComponents/dropDownListBox/dropDownListBoxItem'; +import { IFileService } from 'vs/platform/files/common/files'; export interface SavePlotOptions { uri: string; @@ -40,6 +43,8 @@ const BASE_DPI = 100; // matplotlib default DPI * Show the save plot modal dialog for dynamic plots. * @param layoutService the layout service for the modal * @param keybindingService the keybinding service to intercept shortcuts + * @param dialogService the dialog service to confirm the save + * @param fileService the file service to check if paths exist * @param fileDialogService the file dialog service to prompt where to save the plot * @param plotClient the dynamic plot client to render previews and the final image * @param savePlotCallback the action to take when the dialog closes @@ -48,6 +53,8 @@ const BASE_DPI = 100; // matplotlib default DPI export const showSavePlotModalDialog = ( layoutService: IWorkbenchLayoutService, keybindingService: IKeybindingService, + dialogService: IDialogService, + fileService: IFileService, fileDialogService: IFileDialogService, plotClient: PlotClientInstance, savePlotCallback: (options: SavePlotOptions) => void, @@ -66,7 +73,10 @@ export const showSavePlotModalDialog = ( renderer.render( { - const [path, setPath] = React.useState({ value: props.suggestedPath ?? URI.file(''), valid: true }); + const [directory, setDirectory] = React.useState({ value: props.suggestedPath ?? URI.file(''), valid: true }); + const [name, setName] = React.useState({ value: 'plot', valid: true }); + const [format, setFormat] = React.useState(PlotFormat.PNG); const [width, setWidth] = React.useState({ value: props.plotWidth, valid: true }); const [height, setHeight] = React.useState({ value: props.plotHeight, valid: true }); const [dpi, setDpi] = React.useState({ value: 100, valid: true }); @@ -107,8 +128,8 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { }, [props.plotClient.lastRender?.uri]); const validateInput = React.useCallback((): boolean => { - return path.valid && width.valid && height.valid && dpi.valid; - }, [path, width, height, dpi]); + return directory.valid && width.valid && height.valid && dpi.valid; + }, [directory, width, height, dpi]); React.useEffect(() => { validateInput(); @@ -130,41 +151,58 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { }; const updatePath = (pathString: string) => { - const newPath = URI.file(pathString); - setPath({ value: newPath, valid: !!newPath }); + try { + const newPath = URI.file(pathString); + props.fileService.exists(newPath).then(exists => { + setDirectory({ + value: newPath, valid: exists, + errorMessage: exists ? undefined : localize('positron.savePlotModalDialog.pathDoesNotExist', "Path does not exist.") + }); + }); + } catch (error) { + setDirectory({ value: URI.file(''), valid: false, errorMessage: error.message }); + } }; const browseHandler = async () => { - const uri = await props.fileDialogService.showSaveDialog({ + const uri = await props.fileDialogService.showOpenDialog({ title: localize('positron.savePlotModalDialog.title', "Save Plot"), - filters: filterEntries, + defaultUri: directory.value, + openLabel: localize('positron.savePlotModalDialog.select', "Select"), + canSelectFiles: false, + canSelectFolders: true, + canSelectMany: false, }); - if (uri?.fsPath.length) { - setPath({ value: uri, valid: true }); + if (uri && uri.length > 0) { + updatePath(uri[0].fsPath); } }; const acceptHandler = async () => { if (validateInput()) { - setRendering(true); - - const extension = path.value.fsPath.split('.').pop()?.toLowerCase(); - let format = extension as PlotFormat; - - // default to PNG if the format is unknown - if (!Object.values(PlotFormat).includes(format)) { - format = PlotFormat.PNG; - } - - const plotResult = await generatePreview(format); - - if (plotResult) { - props.savePlotCallback({ uri: plotResult.uri, path: path.value }); + const fileExists = await props.fileService.exists(directory.value); + if (fileExists) { + const confirmation = await props.dialogService.confirm({ + message: localize('positron.savePlotModalDialog.fileExists', "The file already exists. Do you want to overwrite it?"), + primaryButton: localize('positron.savePlotModalDialog.overwrite', "Overwrite"), + cancelButton: localize('positron.savePlotModalDialog.cancel', "Cancel"), + }); + if (!confirmation.confirmed) { + return; + } } + setRendering(true); - setRendering(false); - props.renderer.dispose(); + generatePreview(format) + .then(async (plotResult) => { + const filePath = URI.joinPath(directory.value, `${name.value}.${format}`); + props.savePlotCallback({ uri: plotResult.uri, path: filePath }); + }) + .finally(() => { + setRendering(false); + props.renderer.dispose(); + }); } }; @@ -211,16 +249,46 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => {
localize( - 'positron.savePlotModalDialog.path', - "Path" + 'positron.savePlotModalDialog.directory', + "Directory" ))()} - value={path.value.fsPath} + value={directory.value.fsPath} onChange={e => updatePath(e.target.value)} onBrowse={browseHandler} readOnlyInput={false} - error={!path.valid} + error={!directory.valid} inputRef={inputRef} />
+
+ localize( + 'positron.savePlotModalDialog.name', + "Name" + ))()} + value={name.value} + onChange={e => setName({ value: e.target.value, valid: !!e.target.value })} + error={!name.valid} + /> +
+ +
+
localize( @@ -258,9 +326,9 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { />
- {!path.valid && (() => localize( - 'positron.savePlotModalDialog.noPathMessage', - "Specify a path." + {!directory.valid && (() => localize( + 'positron.savePlotModalDialog.invalidPathError', + "Invalid path: {0}", directory.errorMessage ))()}
diff --git a/src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts b/src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts index 89eb57c9526..f2eaa68e6d4 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts +++ b/src/vs/workbench/contrib/positronPlots/browser/positronPlotsService.ts @@ -24,14 +24,14 @@ import { WebviewPlotClient } from 'vs/workbench/contrib/positronPlots/browser/we import { IPositronNotebookOutputWebviewService } from 'vs/workbench/contrib/positronOutputWebview/browser/notebookOutputWebviewService'; import { IPositronIPyWidgetsService } from 'vs/workbench/services/positronIPyWidgets/common/positronIPyWidgetsService'; import { Schemas } from 'vs/base/common/network'; -import { IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; +import { IDialogService, IFileDialogService } from 'vs/platform/dialogs/common/dialogs'; import { decodeBase64 } from 'vs/base/common/buffer'; import { SavePlotOptions, showSavePlotModalDialog } from 'vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog'; import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; -import { URI } from 'vs/base/common/uri'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService'; +import { localize } from 'vs/nls'; /** The maximum number of recent executions to store. */ const MaxRecentExecutions = 10; @@ -109,7 +109,8 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, @IWorkbenchLayoutService private readonly _layoutService: IWorkbenchLayoutService, @IKeybindingService private readonly _keybindingService: IKeybindingService, - @IClipboardService private _clipboardService: IClipboardService) { + @IClipboardService private _clipboardService: IClipboardService, + @IDialogService private readonly _dialogService: IDialogService) { super(); // Register for language runtime service startups @@ -682,7 +683,7 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe if (this._selectedPlotId) { const plot = this._plots.find(plot => plot.id === this._selectedPlotId); const workspaceFolder = this._workspaceContextService.getWorkspace().folders[0]?.uri; - const suggestedPath = workspaceFolder ? URI.joinPath(workspaceFolder, 'plot.png') : undefined; + const suggestedPath = workspaceFolder ?? undefined; if (plot) { let uri = ''; @@ -692,7 +693,7 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe this.showSavePlotDialog(uri); } else if (plot instanceof PlotClientInstance) { // if it's a dynamic plot, present options dialog - showSavePlotModalDialog(this._layoutService, this._keybindingService, this._fileDialogService, plot, this.savePlotAs, suggestedPath); + showSavePlotModalDialog(this._layoutService, this._keybindingService, this._dialogService, this._fileService, this._fileDialogService, plot, this.savePlotAs, suggestedPath); } else { // if it's a webview plot, do nothing return; @@ -712,7 +713,8 @@ export class PositronPlotsService extends Disposable implements IPositronPlotsSe const data = matches[2]; htmlFileSystemProvider.writeFile(options.path, decodeBase64(data).buffer, { create: true, overwrite: true, unlock: true, atomic: false }) - .then(() => { + .catch((error: Error) => { + this._dialogService.error(localize('positronPlotsService.savePlotError.unknown', 'Error saving plot: {0}', error.message)); }); }; From 5a6897b3960674f378bea14f4fac683eb7599006 Mon Sep 17 00:00:00 2001 From: Tim Mok Date: Mon, 15 Apr 2024 11:05:15 -0400 Subject: [PATCH 9/9] Validate plot name --- .../browser/modalDialogs/savePlotModalDialog.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx index b607fac3114..b52a3165ad4 100644 --- a/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx +++ b/src/vs/workbench/contrib/positronPlots/browser/modalDialogs/savePlotModalDialog.tsx @@ -128,8 +128,8 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { }, [props.plotClient.lastRender?.uri]); const validateInput = React.useCallback((): boolean => { - return directory.valid && width.valid && height.valid && dpi.valid; - }, [directory, width, height, dpi]); + return directory.valid && width.valid && height.valid && dpi.valid && name.valid; + }, [directory, width, height, dpi, name]); React.useEffect(() => { validateInput(); @@ -343,6 +343,12 @@ const SavePlotModalDialog = (props: SavePlotModalDialogProps) => { "DPI must be between 1 and 300." ))()}
+
+ {!name.valid && (() => localize( + 'positron.savePlotModalDialog.invalidNameError', + "Plot name cannot be empty." + ))()} +