Save/Copy Plots #2267
Save/Copy Plots #2267
Conversation
- Relying on the graphical backends turned out to be problamatic because it will need to keep a reference the code which produced the plot. - Even if the complexity of storing the code which produced the plot is acceptable, it won't work for calls to `plot` with mutable data e.g., `plot(1:10, rand(10))`. - Instead save the svg displayed in the plot pane. - !Only tested with `Plots` no other graphical backends
- It's nonfunctional in VSCode and can confuse users.
|
Nice! I'll take a more detailed look tomorrow, but could you please make sure to configure eslint so that it conforms to the code style we have going so far (no semi-colons etc). Really hard to read the diff otherwise (and it would be good to keep the codebase consistent). |
- Documentation for julia-vscode/julia-vscode#2267
pfitzseb
left a comment
pfitzseb
left a comment
There was a problem hiding this comment.
Ok, the code here looks good (although there still are a bunch of weird eslint-looking changes around -- maybe we should just take the time to configure eslint properly and run it on the whole codebase).
That said, I don't think OS specific binaries are the way to go. We really want something that
- Runs in the renderer process (so that it uses the correct clipboard when developing on a remote).
- Doesn't require us to ship binaries.
- No longer needed; the copy feature relies on browser Clipboard API.
- It was needed copy to plots to the clipboard with native binaries; it's no longer needed as the copy feature relies on the browser Clipboard API.
There was a problem hiding this comment.
Looking good and works great! I've left two comments that I'd like to see addressed before merging this though.
Also, there still are a bunch of code style changes in here. Please revert them for now (which should also address the merge conflict); we can figure out a proper eslint setup later on.
src/interactive/plots.ts
Outdated
| */ | ||
| function _writePlotFile(fileName: string, data: FileLike) { | ||
| const rootPath = vscode.workspace.workspaceFolders[0].uri.fsPath | ||
| const plotsDir: string = vscode.workspace |
There was a problem hiding this comment.
Hm, I think we should be using window.showSaveDialog here to prompt for a location; the default path and file name handling here are fine, of course.
- Instead of saving the plots to `julia.plots.path`, enable the user to choose a path from the `SaveDialog`. - Also, remove the default value for `julia.plots.path`, creating the directory if the user don't want it, can be annoying.
AbdulrhmnGhanem
force-pushed
the
export-plot
branch
3 times, most recently
from
ee84fda to
c285887
Compare
AbdulrhmnGhanem
force-pushed
the
export-plot
branch
from
c285887 to
e8aeb05
Compare
|
Reminder to merge the PR here: julia-vscode/docs#35 when this PR gets merged. |
|
I think this just needs a merge conflict resolution? |
davidanthoff
left a comment
davidanthoff
left a comment
There was a problem hiding this comment.
@AbdulrhmnGhanem same thing as with the other one: would be great if you could resolve the merge conflict, and then we can include it in the next minor release.
|
This is included in #2273, so closing. |
4 participants
Add
Save Plotbutton, fixes #424.Add
Copy Plotbutton, fixes #172.plotwith mutable data e.g.,plot(1:10, rand(10)).