Conversation
Callers wanting a one-line data summary under the title previously had to abuse with_title_wrap (which renders both lines at full title size) or place a free TextAnnotation in the plot area. Add Layout::with_subtitle<S: Into<String>>, drawn centred under the title. - Size defaults to round(0.7 * title_size); override with with_subtitle_size. - Colour is derived by muting the title colour toward the background (SUBTITLE_MUTE = 0.4), so it adapts to light and dark themes rather than a fixed grey; colours that can't resolve to RGB (e.g. hsl()) fall back to the title's own colour un-muted. - Wraps independently of the title via with_subtitle_wrap; the global --wrap also covers it. Like the title, it stays on one line unless a wrap width is set. Baselines and the height reserved in ComputedLayout::from_layout use real DejaVu line-height/ascent (matching the title); title_lines is computed once on ComputedLayout and reused. Rendered wherever the title is: via add_labels_and_title for standard and pixel-space plots, propagated to figure panels through clone_layout, and drawn directly in render_jointplot (whose standalone path renders its own title band). Exposed on the CLI as --subtitle and --subtitle-wrap; documented in the layout reference and CLI index. Tested in tests/subtitle.rs and tests/jointplot.rs, and smoke-tested on the scatter and line subcommands.
One more PR from me on the long march to implementing everything I miss from ggplot2. This one is to add a subtitle below the main title, in smaller font and slightly muted... As always, let me know if this isn't something you want to support, or would like to see it done differently! Prepared with my friend Claude. --- Callers wanting a one-line data summary under the title previously had to abuse with_title_wrap (which renders both lines at full title size) or place a free TextAnnotation in the plot area. Add Layout::with_subtitle<S: Into<String>>, drawn centred under the title. - Size defaults to round(0.7 * title_size); override with with_subtitle_size. - Colour is derived by muting the title colour toward the background (SUBTITLE_MUTE = 0.4), so it adapts to light and dark themes rather than a fixed grey; colours that can't resolve to RGB (e.g. hsl()) fall back to the title's own colour un-muted. - Wraps independently of the title via with_subtitle_wrap; the global --wrap also covers it. Like the title, it stays on one line unless a wrap width is set. Baselines and the height reserved in ComputedLayout::from_layout use real DejaVu line-height/ascent (matching the title); title_lines is computed once on ComputedLayout and reused. Rendered wherever the title is: via add_labels_and_title for standard and pixel-space plots, propagated to figure panels through clone_layout, and drawn directly in render_jointplot (whose standalone path renders its own title band). Exposed on the CLI as --subtitle and --subtitle-wrap; documented in the layout reference and CLI index. Tested in tests/subtitle.rs and tests/jointplot.rs, and smoke-tested on the scatter and line subcommands. ## Description <!-- What does this PR do? Why? --> ## Type of change - [ ] New plot type - [x] New feature / API addition - [ ] Bug fix - [ ] Documentation / assets only - [ ] Refactor / housekeeping --- ## Checklist ### Library (new plot type) - [ ] `src/plot/<name>.rs` — struct + builder methods - [ ] `src/plot/mod.rs` — `pub mod` + re-export - [ ] `src/render/plots.rs` — `Plot` enum variant + `bounds()` / `colorbar_info()` / `set_color()` - [ ] `src/render/render.rs` — `render_<name>()`, added to `render_multiple()` match, `skip_axes` if pixel-space - [ ] `src/render/layout.rs` — `auto_from_plots()` extended if categories needed ### Tests - [ ] New test file in `tests/` with ≥ basic render + SVG content + legend tests - [ ] `cargo test --features cli,full` — all existing tests still pass ### CLI (if applicable) - [ ] `src/bin/kuva/<name>.rs` — Args struct (with `/// doc comment`) + `run()` - [ ] `src/bin/kuva/main.rs` — module, Commands variant, match arm - [ ] `scripts/smoke_tests.sh` — at least one invocation - [ ] `tests/cli_basic.rs` — SVG output test + content verification test - [ ] `docs/src/cli/index.md` — subcommand entry - [ ] `man/kuva.1` — regenerated (`./target/debug/kuva man > man/kuva.1`) ### Documentation - [ ] `examples/<name>.rs` — Rust example for doc asset generation - [ ] `scripts/gen_docs.sh` — invocations added; `bash scripts/gen_docs.sh` runs clean - [ ] `docs/src/plots/<name>.md` — documentation page with embedded SVGs - [ ] `docs/src/SUMMARY.md` — link added - [ ] `docs/src/gallery.md` — gallery card added - [ ] `README.md` — plot types table updated ### Visual inspection - [ ] Opened `test_outputs/` — new plot SVGs look correct - [ ] Scanned neighbouring plots in `test_outputs/` for layout regressions - [x] `bash scripts/smoke_tests.sh` — all existing smoke test outputs still look correct - [ ] No text clipped, no legend overlap, no spurious axes on pixel-space plots ### Housekeeping - [x] `CHANGELOG.md` — entry added under `## [Unreleased]` - [ ] `README.md` — item marked done in TODO section if applicable
Keep all-61 structural coverage, typecheck snippets without full features in an isolated package, and limit full-feature trybuild to a small link smoke.
Hello there, I wanted to open this PR because I realized that the output filename can currently conflict with the content Kuva writes. For example, `-o plot.pgn`, an output path without an extension, and even `-o plot.PNG` all succeed but contain SVG. The first two can hide a typo, while the last one looks like a PNG but isn't one. I changed the CLI so `.svg`, `.png`, and `.pdf` are recognized case-insensitively. Other or missing extensions now return an error during argument parsing, before Kuva reads the input or renders anything, and no misleading output file is created. What I changed: - added one shared output-format classifier rather than repeating the rule in each plot command; - used the same classifier for CLI validation and backend selection; - added regression tests for unknown and missing extensions, validation timing, and mixed-case SVG, PNG, and PDF output; - updated the related help text, README, CLI documentation, and changelog entry. I kept the scope limited to CLI file output through `-o`/`--output`. SVG on stdout is unchanged, supported lowercase paths behave as before, and PNG/PDF still require their existing features. This does not add a `--format` flag or change the library backend APIs, terminal output, input handling, or rendering itself. For testing, I first ran the new behaviour tests against the unchanged `dev` implementation and confirmed that the invalid paths were accepted and mixed-case PNG/PDF received SVG. On the updated branch, I ran: - the focused output classifier and CLI tests with both `cli` and `cli,full` features; - `cargo ci-fmt`; - `cargo ci-clippy`; - `cargo ci-test`, including the full CLI suite and doctests; - manual help, failure-side-effect, and `--emit-code` checks. All of these passed. I could not run the mdBook build because `mdbook` is not installed in my local environment, but I reviewed the Markdown changes directly as the source. I have a few questions where I would especially appreciate your preference: 1. Does strict validation for unknown or missing extensions match the behaviour you want, or would you prefer to keep the implicit SVG fallback? 2. I updated the directly related help, README, CLI documentation, and changelog so they match the new behaviour. If you prefer to keep documentation wording and style under your control, I am completely happy to reduce this PR to the code and tests. Thank you for taking a look. ## Type of change - [ ] New plot type - [ ] New feature / API addition - [x] Bug fix - [ ] Documentation / assets only - [ ] Refactor / housekeeping --- ## Checklist ### Library (new plot type) Not applicable — this does not add or change a plot type or public library API. ### Tests - [x] Added focused unit coverage for supported, mixed-case, missing, unsupported, trailing-dot, and non-Unicode extensions - [x] Added CLI coverage for failure diagnostics, absence of output side effects, validation before input reading, and SVG/PNG/PDF content - [x] Focused tests pass with both `cli` and `cli,full` - [x] `cargo ci-test` — the complete suite passed, including 80 CLI tests and 186 doctests - [x] `cargo ci-fmt` - [x] `cargo ci-clippy` ### CLI (if applicable) - [x] Shared `BaseArgs::output` validation and help text updated - [x] Shared backend selection uses the same format classifier - [x] `tests/cli_basic.rs` updated with output and content verification - [x] `docs/src/cli/index.md` updated with the new output contract - [x] `man/kuva.1` scope checked — flattened subcommand output options are not present, so regeneration produces no relevant change - [x] Smoke-test scope checked — no command dispatch or rendering behaviour changed, so no new invocation was added ### Documentation - [x] `README.md` output guidance updated - [x] `docs/src/cli/index.md` output guidance updated - [x] CLI `--help` text updated and inspected - [ ] mdBook build — `mdbook` is not installed in my local environment; changed Markdown was reviewed as source ### Visual inspection Not applicable — this PR does not change plot rendering or layout. Mixed-case PNG and PDF output was validated structurally in the CLI tests. ### Housekeeping - [x] `CHANGELOG.md` entry added under `## [Unreleased]` - [x] Final diff and failure-side-effect checks reviewed Enes K.
### Summary `emit_code_compiles` was compiling all 61 generated programs through trybuild with `cli,full,emit_code`. That passed, but a clean run left about 17G, mostly under trybuild. This PR keeps the broad coverage and changes how the expensive part is done: - all 61 commands still get structural checks from fresh `--emit-code` output - all 61 snippets are generated again and typechecked in a temporary package with `default-features = false` - only `pie` and `surface3d` stay on the full-feature trybuild path as a small link smoke No runtime code, public API, or CLI behaviour changes. ### Why The generated programs use the SVG backend and the normal plotting API. They do not use the optional PNG or PDF backends enabled by `full`. Linking every emitted example against that fuller dependency graph is what blew up the target directory. I also compared a few alternatives afterward. The stronger long-term shape is probably: keep structural checks, typecheck all snippets with `cargo check`, and keep at most one low-debuginfo link smoke. This PR is the small step that fits the current test layout. It is not meant as the final answer to every large debug artifact in CI. ### Result On clean targets here: - focused full-feature emit-code test: about 17G to 3.2G - all-61 minimal-feature check: about 20s, 580M root target; temporary child package removed after the test The remaining 3.2G is still largely the two trybuild cases. I left them in so the first PR stays close to the existing trybuild setup. Happy to remove or replace them in a follow-up if that is preferred. ### Testing ```bash cargo fmt --all -- --check cargo test --locked --features cli,full,emit_code cargo clippy --locked --features cli,full,emit_code -- -D warnings cargo test --locked --test emit_code_minimal_compiles --features cli,emit_code ``` The part I would most like feedback on is the temporary Cargo package used for feature isolation. It keeps the change self-contained and avoids a new workspace member. If there is a cleaner project-local way to express that boundary, I can switch to it.
Fixes 3 issues from a private security disclosure: - SVG attribute injection: group/legend/category labels (data-group, data-x, etc.) were interpolated raw into interactive-mode SVG attributes, letting a crafted data file break out and inject markup. New render_utils::escape_attr, applied at every extra_attrs site in render.rs. - CSS-value injection: Color::Css (the catch-all for unrecognized color strings) and stroke-dasharray/font-family/background-fill in the SVG backend were written unescaped; now funneled through the same escaping. - Terminal escape injection: TerminalBackend::set_char stored raw control characters (ESC, C0/C1, DEL) from data-derived labels, letting --terminal output smuggle ANSI/OSC sequences into the operator's terminal. Now replaced with U+FFFD before entering the char grid. Adds regression tests in tests/svg_attr_escaping.rs and tests/terminal_basic.rs::bar_label_control_chars_are_sanitized.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[0.5.0] — 2026-08-07
Added
svg2pdftokrilla/krilla-svg, plus multi-page output —svg2pdfwas archived upstream by its own maintainer as unmaintained, recommendingkrilla/krilla-svg(same author, used by Typst) as the successor.PdfBackend::render_scenes(&[Scene])and the one-shotrender_to_pdf_multi(pages)render one kuva canvas per PDF page into a single document (like R'spdf()device, e.g. for fgbio/Picard-style reports); nothing is rasterized. By default each page takes its scene's natural size;PdfBackend::with_page_size(PageSize::inches(11.0, 8.5))instead coerces every page to a fixed size, scaling each scene proportionally to fit and centering it, with the scene's own background color filling the letterbox margin. Thepdffeature now requires Rust >= 1.92 (krilla's MSRV) — higher than kuva's own crate-levelrust-version(kept at 1.87 deliberately; see README.md's note on why), tracked separately as[package.metadata.msrv] pdf_featureinCargo.toml. CI'smsrvjob now buildscli,png,embed_font(notcli,full) againstrust-version, plus a newpdf-msrvjob that buildscli,fullagainstpdf_featurespecifically — the pre-existingmsrvjob would otherwise have silently asserted a false claim (thatfullbuilds at 1.87) forever, since nothing else exercises that combination at the older toolchain.PdfBackendis no longer a zero-sized unit struct usable as a bare value (PdfBackend.render_scene(...)) — usePdfBackend::new().kuva twin-yCLI subcommand — dual-axis plot from the command line: two series sharing an x-axis but with independent primary (left) and secondary (right) y-scales, e.g. temperature vs. rainfall. Supportslineandscatteras the plot type on either axis (--primary-type/--secondary-type); per-side color and legend label flags; new shared--y2-label/--y2-min/--y2-max/--log-y2/--y2-tick-formatflags. Closes CLI: Add twin y-plots #106.Layout::with_y2_axis_min/with_y2_axis_max— unconditional secondary-Y-axis bound overrides, mirroringwith_y_axis_min/with_y_axis_max. Found needed while building thetwin-yCLI: the existingwith_y2_rangestill gets nice-rounded and capped near the secondary plots' own data (the same path the auto-computed range takes), so a requested bound far from the data was largely ignored.--x-date-format,--x-date-unit,--x-date-tick-format,--x-date-tick-step) onscatterandline— parses the X column as a date/time using astrftime-style format instead of a plain number, then ticks it with aDateTimeAxis(auto-selected unit/format by default, or an explicit unit with a sensible default tick format, overridable). Closes CLI: Add date/time axes #107.Layout::with_label_background— a semi-opaque background rect behind in-fill value labels (Treemap, Sunburst, Mosaic, Funnel, Gantt), for readability over busy fills or BW-mode hatch patterns. Off by default in color mode; on automatically in BW mode; overridable either way. CLI:--label-background. Closes label backgrounds #102.Layout::with_subtitle— render a secondary line centred under the title for a one-line data summary (e.g.n = 1,234 cells). Sized atround(0.7 × title_size)by default or set explicitly withwith_subtitle_size; coloured by muting the title colour toward the background so it adapts to light and dark themes rather than a fixed grey; word-wrapped independently of the title viawith_subtitle_wrap. The title block reserves the extra height automatically so the plot is pushed down rather than overlapped. CLI:--subtitleand--subtitle-wrapon every subcommand. See Reference → Layout.ParetoPlot— bar chart of category values, sorted descending by default, with a superimposed cumulative-percentage line on a secondary axis (fixed 0-100%, the "80/20 rule" chart). Optional dashed threshold reference line (default 80%, labeled with its percentage) and per-point cumulative-percentage labels. Legend shown by default ("Value" / "Cumulative %"). Secondary-axis ticks are formatted as percentages (0%,20%, …). Categorical axis defaults to rotated (-45°), collision-thinned labels..with_max_categories(n)collapses a long tail of small categories into one stacked "Other" bar, decoded via per-segment legend entries, instead of cluttering the axis..with_horizontal(bool)puts categories on Y and values on X. CLI:kuva pareto.Layout::with_x2_range/with_x2_label/with_log_x2/with_x2_tick_format,ComputedLayout::map_x2) — a top-drawn counterpart to the existing secondary Y-axis (right side), for plots whose secondary encoding pairs with the value axis rather than the category axis (used by horizontalParetoPlot). Third-party plot types can use it directly via the sameLayout/ComputedLayoutfields.--headerflag — forces the first row to be treated as a header even when it looks like data, the explicit counterpart to the existing--no-header. The two are mutually exclusive. Useful for inputs whose column names are all-numeric (e.g. years). Ignored, with a warning, for parquet input (self-describing). Part of CLI: first line as a header #111.Fixed
5,dataover0,1/1,2treated the header row as data and failed withcannot parse 'data' as a number. Detection now also flags a header when any column holds a non-numeric label atop an otherwise all-numeric column. The rule is a strict superset of the old first-cell check, so existing detections are unchanged; genuinely ambiguous inputs are covered by the new--header/ existing--no-headeroverrides.examples/all_plots_simple.rs/all_plots_complex.rs(the "every plot type in one figure" gallery assets) were missingParetoPlot,BandPlot, andLegendPlot— Replaced some plot repeats to include missing plot types.man/kuva.1was missing thetwin-ysubcommand — regenerated (kuva man > man/kuva.1).Scatter3D/Surface3Dinstances combined in one panel now share one 3D coordinate box — each instance previously calleddata_ranges()/drew its own wireframe box independently, so twoScatter3D(or a mix withSurface3D) in the samerender_multiplecall each normalized to their own min/max and could project completely different data onto identical screen coordinates, with the box itself drawn twice.render_multiplenow computes one mergedDataRanges3Dand draws the box once, shared by every 3D instance in the call.Layout::auto_from_twin_y_plots'swith_y2_autounioned the padded x-range across primary and secondary, but leftdata_x_range(the raw extent used by the axis-range capping added for #98) pinned to primary's range alone. When the capped branch triggered, the x-axis max was computed from primary's raw max instead of the true combined max, rounding the axis short and clipping secondary's data past that point.data_x_rangeis now unioned across both series.Security
data-group="...",data-x="...") in interactive-mode output. Previously these were interpolated raw, so a crafted data file's label could break out of the attribute and inject arbitrary markup (a stored-XSS-style issue) into the rendered SVG. Fixed at everyextra_attrscall site insrc/render/render.rsvia a newrender_utils::escape_attr.Color::Css, e.g. an unrecognized--color-byvalue) are now XML-escaped when written intofill/strokeattributes, andstroke-dasharray, rootfont-family/fill, and the background-rectfillin the SVG backend are now escaped as well, closing the same class of attribute-breakout issue for CSS-derived values.--terminaloutput now filters control characters (ESC, C0/C1, DEL) out of data-derived labels before they reach the character grid, replacing them withU+FFFD. Previously a label containing a raw escape sequence (e.g. from an untrusted data file) could be replayed into the operator's real terminal when the rendered grid was printed, potentially triggering an ANSI/OSC-based terminal escape injection.Reported via private disclosure (GHSA-3c48-9r95-hqhr). See the advisory for full details once published.