Add uicolor option

[TheZoq2]

I wanted to embed the flamegraphs in a html document with a dark theme. While setting the background works fine, I couldn't find a way to the text color of the "UI elements", so this is an attempt to add that option.

Here is an example with a dark background but white uicolor.

The search button is quite faint here, maybe I should do something about that

Should I also add these options to the CLI part? For my personal use, the library part is enough, but I suppose others may want it via CLI.

I'm very unfamiliar with this code base, so let me know if this is not the right way to do it. Also, feel free to disregard if this is out of scope

[TheZoq2]

Updated to also pass tests now. I hope I did that part correctly.

Should I also add a test to make sure that these style options are applied?

[jonhoo]

Thanks for taking the time to contribute!

You're on the right track, though I wonder if we should make the change in a slightly different place (see comments). And yes, a test would be great!

Separately, you may also want to add a command-line argument option for this in src/bin/flamegraph.rs.

[TheZoq2]

After adding the binary flag in the latest commit, this is now failing on my local machine with

---- collapse::dtrace::tests::test_collapse_multi_dtrace_simple stdout ----
thread 'collapse::dtrace::tests::test_collapse_multi_dtrace_simple' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/test/src/lib.rs:184:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- collapse::dtrace::tests::test_collapse_multi_dtrace stdout ----
thread 'collapse::dtrace::tests::test_collapse_multi_dtrace' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/test/src/lib.rs:184:5

---- collapse::perf::tests::test_collapse_multi_perf stdout ----
thread 'collapse::perf::tests::test_collapse_multi_perf' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/test/src/lib.rs:184:5

---- collapse::perf::tests::test_collapse_multi_perf_simple stdout ----
thread 'collapse::perf::tests::test_collapse_multi_perf_simple' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/test/src/lib.rs:184:5

It feels like those things shouldn't be related, right?

[jonhoo]

It feels like those things shouldn't be related, right?

I agree. Our tests don't generally fail spuriously so it's a bit concerning, but unlikely to be related to your change 🤔

Also, looks like there's a conflict with main

[jonhoo]

Let's be a little more helpful here. Something along the lines of "expected a color in hexadecimal format"

[TheZoq2]

Turns out the reason those tests were failing was that I forgot git update --init 🙂

The UiColor struct was not intentional, I initially started out trying to use it instead of just using Option<Color>. Then I accidentally replaced the wrong thing with the LSP before getting rid of its use. Should be better now

[jonhoo]

Oh, and would you mind also updating the CHANGELOG file?

[TheZoq2]

Updated with all three changes

[codecov]

Codecov Report

Base: 90.32% // Head: 90.28% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (a835462) compared to base (eeea9f5).
Patch coverage: 96.29% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #271      +/-   ##
==========================================
- Coverage   90.32%   90.28%   -0.04%     
==========================================
  Files          19       19              
  Lines        4226     4219       -7     
==========================================
- Hits         3817     3809       -8     
- Misses        409      410       +1     

Impacted Files	Coverage Δ
src/flamegraph/color/mod.rs	85.71% <92.30%> (-0.85%)	⬇️
src/flamegraph/mod.rs	97.95% <100.00%> (+0.01%)	⬆️
src/flamegraph/svg.rs	97.79% <100.00%> (+0.03%)	⬆️
src/collapse/common.rs	66.66% <0.00%> (-0.22%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jonhoo]

value_parser was added in clap 3.2.0, so you'll have to update Cargo.toml to have that be the required version, then this should work 👍

[jonhoo]

Published as 0.11.13 🎉