fix #1141

[ulysses4ever]

Fixes #1141

Contributor checklist:

• I've updated the documentation to reflect these changes
• I've added an entry to NEWS.md
• I've added and/or updated the unit tests
• I've run the regression tests
• I've squash'ed or fixup'ed junk commits with git-rebase
• I've built the docs and confirmed these changes don't cause new errors

[codecov-io]

Codecov Report

Merging #1142 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1142   +/-   ##
=======================================
  Coverage   87.23%   87.23%           
=======================================
  Files          32       32           
  Lines        3956     3956           
=======================================
  Hits         3451     3451           
  Misses        505      505

Impacted Files	Coverage Δ
src/geom/bar.jl	93.33% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9a32e3...59d5614. Read the comment docs.

[bjarthur]

looks great. i would suggest to include your broken example plot as a new unit test, or in your running of the regression tests did you find something equivalent? if the latter, which one? and thanks for taking the time to contribute!

[bjarthur]

I don't see any horizontal bar charts with spacing in the test suite.

I'm I right that in order to add test case I should just add a script with plot inside in
test/testscripts?

correct!

[ulysses4ever]

Thanks! I've just found out that I ran test suite wrong (did Pkg.test once). So I'm going to rerun it properly and try to make sense from the compare_examples.jl's output. Also I'm not sure if Julia indeed recompiles Gadfly when it switches between git-branches.

[tlnagy]

When you switch branches in the Gadfly folder (e.g. ~/.julia/v0.6/Gadfly/), julia will automatically recompiling Gadfly when you do Pkg.test("Gadfly"). You check and make sure by running using Gadfly in the REPL right after switching the branch and you should see something like INFO: Recompiling Gadfly

[ulysses4ever]

Tests are failing with something strange

~/.julia/v0.6/Gadfly$ julia test/compare_examples.jl --bw .js.svg
...
Comparing density.js.svg ... same!
Comparing discrete_bar_horizontal.js.svg ... different :(
Couldn't get a file descriptor referring to the console
ERROR: LoadError: failed process: Process(`open /home/ulysses/.julia/v0.6/Gadfly/test/diffedoutput/discrete_bar_horizontal.js.svg.png`, ProcessExited(1)) [1]
Stacktrace:
 [1] pipeline_error(::Base.Process) at ./process.jl:682
 [2] run(::Cmd) at ./process.jl:651
 [3] macro expansion at /home/ulysses/.julia/v0.6/Gadfly/test/compare_examples.jl:115 [inlined]
 [4] anonymous at ./<missing>:?
 [5] include_from_node1(::String) at ./loading.jl:576
 [6] include(::String) at ./sysimg.jl:14
 [7] process_options(::Base.JLOptions) at ./client.jl:305
 [8] _start() at ./client.jl:371
while loading /home/ulysses/.julia/v0.6/Gadfly/test/compare_examples.jl, in expression starting on line 69

[bjarthur]

are you on mac, windows, or linux? compare_examples calls the system open command on the png files, which displays them with your default image viewer.

[ulysses4ever]

Ubuntu Linux. I thought open is Mac-only thingy, isn't it?

[ulysses4ever]

It seems for Linux distros you could try xdg-open, and it seems, Windows is out of luck.

[ulysses4ever]

Here we are: only one chart has a diff after my fix, and this is, unsurprisingly, discrete_bar_horizontal.jl. Here is the image of the diff: on the right is “gennedoutput” (my fixed version), on the left is master (“cachedoutput”).

BTW: names cached/genned are hardly make much sense to me. I'd change for something like master/devel. But this is a matter for other issue, I guess…

[ulysses4ever]

(Sorry for the noise above) Added test case for horizontal bar chart with spacing.

[bjarthur]

looks great! i will merge once the CI finishes. for some reason it didn't run.

will also work on a PR for the two issues you mentioned above for compare_examples. thanks for catching this.

[ulysses4ever]

I think CI was confused by my rebase and consequent push -f when adding test case. Good to know you actually ran it.

I was about to fix the issue with open actually. But if you already doing this, that's fine.

Thanks for the great library!

[bjarthur]

if you've already got a fix for open i'd say go ahead and submit a PR as i haven't started yet and am pretty busy with #1116. i'd suggest using the already existing Gadfly.open_file.

i liked your suggestion to rename cachedoutput to master-output and gennedoutput to devel-output. that could go in the same PR as a second commit.

[ulysses4ever]

Sure, give me a couple of days.