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

VSCode interacts poorly with Gnuplot? #45

Open
PetrKryslUCSD opened this issue Jul 28, 2021 · 15 comments
Open

VSCode interacts poorly with Gnuplot? #45

PetrKryslUCSD opened this issue Jul 28, 2021 · 15 comments

Comments

@PetrKryslUCSD
Copy link

Please refer to
https://discourse.julialang.org/t/gnuplot-from-vscode-no-plot/65458

@wentasah
Copy link
Contributor

What is your Gnuplot.jl version? If 1.4, it should work (see #43). If you use an earlier version, can you try #31 (comment)?

@lazarusA
Copy link

lazarusA commented Jul 28, 2021

it works just fine for me. And indeed its version 1.4.

@PetrKryslUCSD
Copy link
Author

PetrKryslUCSD commented Jul 28, 2021

I am on 1.4.0. Plotting with this Gnuplot.jl works fine from Sublime Text, but not from VSCode.

@PetrKryslUCSD
Copy link
Author

With these settings I get the plot in the plot pane:
image
When I turn off these check boxes, I get nothing. No error message, no output.

@wentasah
Copy link
Contributor

@PetrKryslUCSD Do I understand correctly, that you had Use Plot Pane switched off? If yes, then I think the following change should fix the problem (I'll send PR later):

diff --git a/src/Gnuplot.jl b/src/Gnuplot.jl
index 18a8057..78a43c0 100644
--- a/src/Gnuplot.jl
+++ b/src/Gnuplot.jl
@@ -241,7 +241,7 @@ function __init__()
     options.gpviewer = !(
         ((isdefined(Main, :IJulia)  &&  Main.IJulia.inited)  ||
          (isdefined(Main, :Juno)    &&  Main.Juno.isactive()) ||
-         (isdefined(Main, :VSCodeServer)) ||
+         (isdefined(Main, :VSCodeServer) && Main.VSCodeServer.PLOT_PANE_ENABLED[]) ||
          (isdefined(Main, :PlutoRunner)) )
     )
     if isdefined(Main, :VSCodeServer)

The problem with this solution is, that the check for enabled plot panel is executed only when loading Gnuplot.jl. If that settings is changed later, no plots will be shown. However, this seems to be standard behavior of VS Code Julia support. When I try to plot something with Plots and have plot pane disabled, the result is the same - no plot and no error.

@PetrKryslUCSD
Copy link
Author

@wentasah I will try it when it lands. Thanks.

@gcalderone
Copy link
Owner

Thank you @PetrKryslUCSD for reporting the issue and @wentasah for proposing a solution.
Indeed the problem seems to be the setting of Gnuplot.options.gpviewer (further info here).

Please keep in mind that Gnuplot.jl is primarily targeted to use gnuplot terminal windows (Gnuplot.options.gpviewer = true) rather than the display() interface used in VSCode, Juno, Jupyter, etc.,. Reasons are: gnuplot windows are typically faster, interactive, unlimited in number, and the only relevant configuration is the gnuplot terminal one, making the behaviour easily reproducible. On the other hand the plot pane (used in VSCode, Juno, Jupyter, etc.,) is typically limited to one window, with limited interactivity, and hardly foreseeable interplay between gnuplot terminal and IDE settings.

wentasah added a commit to wentasah/Gnuplot.jl that referenced this issue Jul 29, 2021
When the plot pane is disabled, VS Code neither shows the plot nor
prints any warning/error. The plot is not shown because Gnuplot.jl
thinks the VS Code can display it. We fix that by adding a check for
whether the pane is enabled.

This is not an ideal solution because the check is executed only when
loading Gnuplot.jl. If the panel is disabled later in the REPL
session, no plots will be shown. If this turns out to be a problem, we
will need to extend `showable` and perform the check there.

The problem of not showing a warning/error when the plot pane is
disabled must be addressed in julia-vscode. It seems that the Plots
package doesn't produce any warning/error in this case too.

Related to gcalderone#45.
@wentasah
Copy link
Contributor

@gcalderone I agree with what you wrote and I personally don't use VS Code and internal viewers.

My motivation for doing this integration is to make it easier for newcomers (typically students) to start using Gnuplot.jl and Julia in general. I believe that it's beneficial if Gnuplot.jl behaves under VS Code similarly as other plotting packages. For example, now I see, that VS Code has a "Plot navigator" pane, which shows a history of past plots and allows to navigate between them. I can imagine some people would prefer this working style instead of multiple gnuplot windows.

On the other hand, those people may never learn that there is another (in some cases better) way of using Gnuplot. The good think is that it's easy to switch between those ways by toggling gpviewer and this is well documented.

@pfitzseb
Copy link

I don't understand why this package needs to check whether it's running inside of VSCode. It would be best if it just used the normal Julia display machinery (register a GunplotDisplay and define the appropriate display methods) to display its figures.

@wentasah
Copy link
Contributor

wentasah commented Jul 29, 2021

Because it's just an interface to an external program, which uses its own way of displaying plots. And some people prefer this because it's fast, interactive (zoom, pan) etc. The plots can be shown even if you work in the plain text REPL. So you want to use the Julia display machinery only if it's really available.

@pfitzseb
Copy link

That's an implementation detail though. If a package is supposed to work well with the rest of the Julia ecosystem, and that includes how display works. Of course you're free to ignore that, but then it doesn't make sense to add special casing for a bunch of frontends.

@gcalderone
Copy link
Owner

As explained above using the display machinery is definitely not the main purpose of Gnuplot.jl, and I believe it works better without it (or at least it fits better on my workflow). Still, as mentioned by @wentasah, such functionalities turn out to be useful in some case (newcomers, students, persons used to work within IDEs).
So the functionalities are currently offered as a best effort.

Of course it would be much better to provide them through dedicated packages but this would require too much effort on my side. I would be happy to give away the display-related code from Gnuplot.jl to some GnuplotOnVSCode.jl package, if someone is willing to take the committment...

@wentasah
Copy link
Contributor

It would probably make sense to avoid this special-casing and use displayable(MIME"image/svg+xml"()) to detect the display capability. @gcalderone Would this work for you? However, currently VS Code returns true even if the "Use Plot Pane" settings is off, which basically means that we have to special case anyway to prevent issues like this one. Also, I'm not sure about the other environments that are currently special-cased.

@wentasah
Copy link
Contributor

Another possibility to avoid the special-casing would be to introduce GnuplotDisplay, a subtype of AbstractDisplay, which would send the plot to the external gnuplot process in the same way as it is done now in show(). I'll try to prototype it to see whether it's a good idea or not.

Currently it seems, that with GnuplotDisplay standard Julia mechanisms could be used for management of displays. Setting gpviewer would influence just the behavior of GnuplotDisplay and not the whole package. This would make Gnuplot.jl more compatible with other packages - not only VSCode. For example, recently, I played with Weave.jl and I needed a hackish workaround in Gnuplot.jl to make it work with Weave.jl. It might be that introduction of GnuplotDisplay would help is this case as well.

@pfitzseb
Copy link

pfitzseb commented Jul 29, 2021

Another possibility to avoid the special-casing would be to introduce GnuplotDisplay

Yes, that's what I meant above. I did try my hand at a implementation for that, but couldn't figure out how to actually display something with this package :P But in theory something like

diff --git a/src/Gnuplot.jl b/src/Gnuplot.jl
index 18a8057..aa8a8a1 100644
--- a/src/Gnuplot.jl
+++ b/src/Gnuplot.jl
@@ -236,14 +236,7 @@ const sessions = OrderedDict{Symbol, Session}()
 const options = Options()
 
 function __init__()
-    # Check whether we are running in an IJulia, Juno, VSCode or Pluto session.
-    # (copied from Gaston.jl).
-    options.gpviewer = !(
-        ((isdefined(Main, :IJulia)  &&  Main.IJulia.inited)  ||
-         (isdefined(Main, :Juno)    &&  Main.Juno.isactive()) ||
-         (isdefined(Main, :VSCodeServer)) ||
-         (isdefined(Main, :PlutoRunner)) )
-    )
+    push!(Base.Multimedia.displays, GnuplotDisplay())
     if isdefined(Main, :VSCodeServer)
         # VS Code shows "dynamic" plots with fixed and small size :-(
         options.mime[MIME"image/svg+xml"] = replace(options.mime[MIME"image/svg+xml"], "dynamic" => "")
@@ -1333,7 +1326,7 @@ function driver(_args...; is3d=false)
     end
 
     if options.gpviewer  &&  doDump
-        execall(gp)
+        # execall(gp)
     end
     return SessionID(gp.sid, doDump)
 end
@@ -1551,6 +1544,12 @@ end
 # ╰───────────────────────────────────────────────────────────────────╯
 # --------------------------------------------------------------------
 
+struct GnuplotDisplay <: AbstractDisplay end
+
+function Base.display(::GnuplotDisplay, obj::SessionID)
+    # implement me!
+end
+
 function show(obj::SessionID)
     gp = getsession(obj.sid)
     @info "Gnuplot session" sid=gp.sid datasets=length(gp.datas) plots=length(gp.plots)

should be enough.

wentasah added a commit to wentasah/Gnuplot.jl that referenced this issue Jul 30, 2021
This should ensure better compatibility with the Julia package
ecosystem. Now, if Gnuplot.jl is used in an environment capable of
showing multimedia content (IJulia, VS Code, Pluto), this will take
precedence over using gnuplot's built-in viewer. In the REPL, gnuplot
viewer is used by default.

In VS Code, For example, when the "Use Plot Pane" settings is enabled,
the plots show in that pane, but when it is disabled, gnuplot viewer
is automatically used.

For people who prefer to always use the gnuplot viewer, they can
achieve that by running:

    popdisplay(Gnuplot.GnuplotDisplay())
    pushdisplay(Gnuplot.GnuplotDisplay())

This will ensure that the gnuplot display has the highest priority and
will be used even in environments such as IJulia.

On the other hand, to disable the gnuplot viewer, one can run:

    Gnuplot.option.gpviewer = false

What was tested:
- [x] Plotting from REPL
- [x] VS Code
  - [x] With Plot Pane enabled
  - [x] With Plot Pane disabled
- [x] IJulia
- [ ] Gnuplot.jl test suite
- [ ] Pluto
- [ ] Juno
- [ ] DrySessions

Related to gcalderone#45
wentasah added a commit to wentasah/Gnuplot.jl that referenced this issue Jul 30, 2021
This should ensure better compatibility with the Julia package
ecosystem. Now, if Gnuplot.jl is used in an environment capable of
showing multimedia content (IJulia, VS Code, Pluto), this will take
precedence over using gnuplot's built-in viewer. In the REPL, gnuplot
viewer is used by default.

In VS Code, For example, when the "Use Plot Pane" settings is enabled,
the plots show in that pane, but when it is disabled, gnuplot viewer
is automatically used.

For people who prefer to always use the gnuplot viewer, they can
achieve that by running:

    popdisplay(Gnuplot.GnuplotDisplay())
    pushdisplay(Gnuplot.GnuplotDisplay())

This will ensure that the gnuplot display has the highest priority and
will be used even in environments such as IJulia.

On the other hand, to disable the gnuplot viewer, one can run:

    Gnuplot.option.gpviewer = false

What was tested:
- [x] Plotting from REPL
- [x] VS Code
  - [x] With Plot Pane enabled
  - [x] With Plot Pane disabled
- [x] IJulia
- [ ] Gnuplot.jl test suite
- [ ] DrySessions
- [ ] Pluto
- [ ] Juno
- [ ] Weave.jl

Related to gcalderone#45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants