add spinner option to ProgressUnknown

[stevengj]

This adds a spinner=true option to the ProgressUnknown case, mimicking the one in Pkg (though you can also customize the spinner characters), inspired by this discourse thread.

Closes #92.

[codecov[bot]]

Codecov Report

Merging #206 (5c4b840) into master (aae026e) will increase coverage by 0.07%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #206      +/-   ##
==========================================
+ Coverage   96.32%   96.40%   +0.07%     
==========================================
  Files           1        1              
  Lines         490      500      +10     
==========================================
+ Hits          472      482      +10     
  Misses         18       18              

Impacted Files	Coverage Δ
src/ProgressMeter.jl	96.40% <100.00%> (+0.07%)	:arrow_up:

---

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 aae026e...5c4b840. Read the comment docs.

[stevengj]

Looks like an unrelated test failure popping up randomly (only occurring on some commits):

Core: Test Failed at /Users/runner/work/ProgressMeter.jl/ProgressMeter.jl/test/core.jl:50
  Expression: #= /Users/runner/work/ProgressMeter.jl/ProgressMeter.jl/test/core.jl:50 =# @elapsed(prog_perf(10 ^ 7)) < 9 * #= /Users/runner/work/ProgressMeter.jl/ProgressMeter.jl/test/core.jl:50 =# @elapsed(noprog_perf(10 ^ 7))
   Evaluated: 0.533898558 < 0.526445748

Since this is a timing comparison, maybe it's getting broken by some performance variation on the CI machines?

[IanButterworth]

Indeed. It can be relaxed. I thought I found a safe spot after some recent optimization but CI does indeed sometimes really run slow. Feel free to increase the multiplier.

This is a nice addition btw 👍

[stevengj]

@IanButterworth, I made a separate PR to disable the timing test when running in Github CI. If you want to merge that one first, I can rebase this PR.

[IanButterworth]

Seems fair. I have seen CI go incredibly slow and still complete.

The other PR is now merged

[stevengj]

Thanks, rebased.

[stevengj]

Should be ready to merge if approved?

[IanButterworth]

Looks good to me!

For maximum emoji expression..

begin
    using ProgressMeter, REPL
    prog = ProgressUnknown("Searching:", spinner=true)
    spinner = join(values(REPL.REPLCompletions.emoji_symbols))
    while true
       ProgressMeter.next!(prog, spinner=spinner)
       rand(1:10^8) == 0xB00 && break
    end
    ProgressMeter.finish!(prog)
end

[stevengj]

(With such a long list of symbols, I would recommend a Vector{Char} instead of a String, so that it has random access: spinner = first.(values(REPL.REPLCompletions.emoji_symbols)).)

[KristofferC]

It is a bit surprising that ProgressMeter.finish!(prog; spinner="ok") does not work as one thinks it would.

[stevengj]

What would you expect spinner="ok" to do?

[KristofferC]

Set the spinner to "ok" when finishing? Or more concretely, if I pass a string with ANSI codes that sets a color on the finalizing spinner, I would want that to be used.

[stevengj]

Oh, I see — yes, we don't currently support a string on completion. Would be easy to add this, presumably.