From 4e5a437a54fe6b06584caba15c714ad59c613faf Mon Sep 17 00:00:00 2001 From: JetSquirrel Date: Tue, 29 Sep 2026 19:04:25 +0800 Subject: [PATCH 1/3] Open documents Finder hands the app "Open With", a double-clicked .dash and a drop on the Dock icon arrive as file:// URLs through the platform's openURLs callback, on launch and while running. The callback has no App context, so it queues the decoded paths and the root view drains them on a short poll, opening them the way a drop on the window does: an app folder as an app tab, a .dash as a dashboard, anything else as data. Info.plist declares the .dash type so Finder offers DuckLocal for it. Co-Authored-By: Claude Opus 5.5 (1M context) --- assets/Info.plist | 35 +++++++++++++ src/app.rs | 127 ++++++++++++++++++++++++++++++++++++++++++---- src/main.rs | 13 +++-- 3 files changed, 163 insertions(+), 12 deletions(-) diff --git a/assets/Info.plist b/assets/Info.plist index 911a2f0..8a212ae 100644 --- a/assets/Info.plist +++ b/assets/Info.plist @@ -22,5 +22,40 @@ 12.0 NSHighResolutionCapable + UTExportedTypeDeclarations + + + UTTypeIdentifier + com.ducklocal.dash + UTTypeDescription + DuckLocal Dashboard Spec + UTTypeConformsTo + + public.text + + UTTypeTagSpecification + + public.filename-extension + + dash + + + + + CFBundleDocumentTypes + + + CFBundleTypeName + DuckLocal Dashboard + CFBundleTypeRole + Editor + LSHandlerRank + Owner + LSItemContentTypes + + com.ducklocal.dash + + + diff --git a/src/app.rs b/src/app.rs index b1b0404..25c99bd 100644 --- a/src/app.rs +++ b/src/app.rs @@ -8,6 +8,54 @@ use gpui_kit::component::resizable::{h_resizable, resizable_panel}; use gpui_kit::component::{v_flex, ActiveTheme, Sizable, WindowExt}; use gpui_kit::prelude::FluentBuilder; use gpui_kit::*; +use std::sync::{Mutex, PoisonError}; +use std::time::Duration; + +/// Documents Finder asks the app to open — "Open With", a double-clicked +/// `.dash`, a drop on the Dock icon — while it runs or as it launches. The +/// platform's openURLs callback gets no App context, so it queues the paths +/// here and the root view drains them once a frame is up. +static FINDER_OPENS: Mutex> = Mutex::new(Vec::new()); + +/// Queue one URL from the platform's openURLs callback. Only `file://` URLs +/// are documents; anything else was not meant for us. +pub(crate) fn queue_finder_open(url: &str) { + if let Some(path) = file_url_path(url) { + FINDER_OPENS + .lock() + .unwrap_or_else(PoisonError::into_inner) + .push(path); + } +} + +/// The path a `file://` URL points at. Percent-escapes are UTF-8; the +/// authority is empty or `localhost` for a local file. +fn file_url_path(url: &str) -> Option { + fn hex(byte: u8) -> Option { + match byte { + b'0'..=b'9' => Some(byte - b'0'), + b'a'..=b'f' => Some(byte - b'a' + 10), + b'A'..=b'F' => Some(byte - b'A' + 10), + _ => None, + } + } + + let authority_and_path = url.strip_prefix("file://")?; + let path_start = authority_and_path.find('/')?; + let encoded = &authority_and_path[path_start..]; + let mut decoded = Vec::with_capacity(encoded.len()); + let mut bytes = encoded.bytes(); + while let Some(byte) = bytes.next() { + if byte == b'%' { + let hi = hex(bytes.next()?)?; + let lo = hex(bytes.next()?)?; + decoded.push(hi << 4 | lo); + } else { + decoded.push(byte); + } + } + String::from_utf8(decoded).ok() +} use crate::analysis::apps; use crate::i18n::{tr, trf}; @@ -134,6 +182,31 @@ impl DuckLocalApp { }) .detach(); + // Finder opens queue from the moment the platform callback is + // registered — possibly before this view exists — so drain on a slow + // poll and open them like a drop on the window. The task ends with + // the window: `update_in` fails once the view is gone. + cx.spawn_in(window, async move |this, cx| { + loop { + smol::Timer::after(Duration::from_millis(200)).await; + let paths: Vec = std::mem::take( + &mut *FINDER_OPENS + .lock() + .unwrap_or_else(PoisonError::into_inner), + ); + if paths.is_empty() { + continue; + } + if this + .update_in(cx, |this, window, cx| this.open_external(paths, window, cx)) + .is_err() + { + break; + } + } + }) + .detach(); + Self { state, title_bar, @@ -143,15 +216,11 @@ impl DuckLocalApp { } } - /// Paths dropped on the window: an app directory opens an app tab, a - /// `.dash` file a dashboard tab, and everything else is the same request - /// the command line and the pickers make. - fn drop_paths(&mut self, paths: &ExternalPaths, window: &mut Window, cx: &mut Context) { - let requested: Vec = paths - .paths() - .iter() - .map(|path| path.to_string_lossy().to_string()) - .collect(); + /// Paths from outside the app — a drop on the window, or a document + /// Finder opens with it: an app directory opens an app tab, a `.dash` + /// file a dashboard tab, and everything else is the same request the + /// command line and the pickers make. + fn open_external(&mut self, requested: Vec, window: &mut Window, cx: &mut Context) { let (directories, rest) = apps::split_paths(&requested); for directory in directories { self.workspace @@ -166,6 +235,15 @@ impl DuckLocalApp { open_paths(self.state.clone(), data, window, cx); } } + + fn drop_paths(&mut self, paths: &ExternalPaths, window: &mut Window, cx: &mut Context) { + let requested: Vec = paths + .paths() + .iter() + .map(|path| path.to_string_lossy().to_string()) + .collect(); + self.open_external(requested, window, cx); + } } impl Render for DuckLocalApp { @@ -231,3 +309,34 @@ impl Render for DuckLocalApp { .child(self.status_bar.clone()) } } + +#[cfg(test)] +mod tests { + // Deliberately not `use super::*`: that pulls in `gpui_kit::*`, whose + // `test` macro shadows the built-in `#[test]`. + use super::file_url_path; + + #[test] + fn file_urls_decode_to_paths() { + assert_eq!( + file_url_path("file:///Users/admin/aiops2_overview.dash"), + Some("/Users/admin/aiops2_overview.dash".to_string()) + ); + // Spaces and CJK arrive percent-encoded. + assert_eq!( + file_url_path("file:///Users/admin/My%20Reports/%E6%97%A5%E6%8A%A5.dash"), + Some("/Users/admin/My Reports/日报.dash".to_string()) + ); + assert_eq!( + file_url_path("file://localhost/Users/admin/x.dash"), + Some("/Users/admin/x.dash".to_string()) + ); + } + + #[test] + fn non_file_urls_are_not_documents() { + assert_eq!(file_url_path("https://example.com/x.dash"), None); + assert_eq!(file_url_path("file://"), None); + assert_eq!(file_url_path("file:///bad%zz"), None); + } +} diff --git a/src/main.rs b/src/main.rs index 6f9cf15..535c7f1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -58,9 +58,16 @@ fn main() { .filter(|arg| !arg.starts_with("-psn_")) .collect(); - gpui_kit::application() - .with_assets(assets::AppAssets) - .run(move |cx| { + let application = gpui_kit::application().with_assets(assets::AppAssets); + // Finder's "Open With" arrives here as file:// URLs — on launch and on + // every later open — and the callback gets no App context, so the paths + // are queued for the root view to drain (app.rs). + application.on_open_urls(|urls| { + for url in urls { + crate::app::queue_finder_open(&url); + } + }); + application.run(move |cx| { gpui_kit::init(cx); ui::init(cx); Theme::change(ThemeMode::Light, None, cx); From ad098e223f1b5fc88f7b09d644f4ace8f3113276 Mon Sep 17 00:00:00 2001 From: JetSquirrel Date: Tue, 29 Sep 2026 19:04:25 +0800 Subject: [PATCH 2/3] Pin gpui-kit by rev again, so the dependency check passes The v0.7.0 upgrade pinned the four gpui-kit crates by tag, and scripts/check-deps.sh only recognises rev pins, so CI has failed on it since. Pin the commit the tag names, 0c830f4d: the same code, in the form the check enforces. Co-Authored-By: Claude Opus 5.5 (1M context) --- Cargo.lock | 22 +++++++++++----------- Cargo.toml | 8 ++++---- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b33daa9..a83da54 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3173,7 +3173,7 @@ dependencies = [ [[package]] name = "gpui-base" version = "0.7.0" -source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" +source = "git+https://github.com/longbridge/gpui-kit?rev=0c830f4d257e69fdd17200650533ab4ca9a40cc0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "aho-corasick", "anyhow", @@ -3212,7 +3212,7 @@ dependencies = [ [[package]] name = "gpui-component" version = "0.7.0" -source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" +source = "git+https://github.com/longbridge/gpui-kit?rev=0c830f4d257e69fdd17200650533ab4ca9a40cc0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "anyhow", "chrono", @@ -3259,7 +3259,7 @@ dependencies = [ [[package]] name = "gpui-component-macros" version = "0.7.0" -source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" +source = "git+https://github.com/longbridge/gpui-kit?rev=0c830f4d257e69fdd17200650533ab4ca9a40cc0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -3270,7 +3270,7 @@ dependencies = [ [[package]] name = "gpui-component-shell" version = "0.7.0" -source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" +source = "git+https://github.com/longbridge/gpui-kit?rev=0c830f4d257e69fdd17200650533ab4ca9a40cc0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "gpui-component", "gpui-shell", @@ -3279,7 +3279,7 @@ dependencies = [ [[package]] name = "gpui-fps" version = "0.7.0" -source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" +source = "git+https://github.com/longbridge/gpui-kit?rev=0c830f4d257e69fdd17200650533ab4ca9a40cc0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "core-graphics 0.24.0", "gpui-pre", @@ -3296,7 +3296,7 @@ dependencies = [ [[package]] name = "gpui-kit" version = "0.7.0" -source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" +source = "git+https://github.com/longbridge/gpui-kit?rev=0c830f4d257e69fdd17200650533ab4ca9a40cc0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "gpui-base", "gpui-component", @@ -3309,7 +3309,7 @@ dependencies = [ [[package]] name = "gpui-kit-assets" version = "0.7.0" -source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" +source = "git+https://github.com/longbridge/gpui-kit?rev=0c830f4d257e69fdd17200650533ab4ca9a40cc0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "anyhow", "gpui-pre", @@ -3832,7 +3832,7 @@ checksum = "829258540ea51bb9ea4a71686beffe5c99586901ba9f39944d06e4a180ab63fe" [[package]] name = "gpui-shell" version = "0.7.0" -source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" +source = "git+https://github.com/longbridge/gpui-kit?rev=0c830f4d257e69fdd17200650533ab4ca9a40cc0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "anyhow", "async-channel", @@ -4222,7 +4222,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.62.2", + "windows-core 0.57.0", ] [[package]] @@ -7437,7 +7437,7 @@ dependencies = [ [[package]] name = "rquickjs" version = "0.12.9" -source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" +source = "git+https://github.com/longbridge/gpui-kit?rev=0c830f4d257e69fdd17200650533ab4ca9a40cc0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "quickjs-jit", ] @@ -10005,7 +10005,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index ea19ded..0297eef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,9 +17,9 @@ categories = ["database", "gui"] # `gpui-base` in the build. # `tree-sitter-sql`: the SQL editor and the SQL in `.dash` heredocs are the only # code DuckLocal highlights, so it links that one grammar rather than the set. -gpui-kit = { git = "https://github.com/longbridge/gpui-kit", tag = "v0.7.0", features = ["tree-sitter-sql"] } -gpui-shell = { git = "https://github.com/longbridge/gpui-kit", tag = "v0.7.0" } -gpui-component-shell = { git = "https://github.com/longbridge/gpui-kit", tag = "v0.7.0" } +gpui-kit = { git = "https://github.com/longbridge/gpui-kit", rev = "0c830f4d257e69fdd17200650533ab4ca9a40cc0", features = ["tree-sitter-sql"] } +gpui-shell = { git = "https://github.com/longbridge/gpui-kit", rev = "0c830f4d257e69fdd17200650533ab4ca9a40cc0" } +gpui-component-shell = { git = "https://github.com/longbridge/gpui-kit", rev = "0c830f4d257e69fdd17200650533ab4ca9a40cc0" } duckdb = { version = "1", features = ["bundled", "json", "parquet"] } calamine = { version = "0.36", features = ["dates"] } smol = "2" @@ -45,7 +45,7 @@ rust_xlsxwriter = "0.99" # `gpui-shell` scripts run on the QuickJS fork the toolkit pins; the crates.io # `rquickjs` would be a second, incompatible copy of the same crate. [patch.crates-io] -rquickjs = { git = "https://github.com/longbridge/gpui-kit", tag = "v0.7.0" } +rquickjs = { git = "https://github.com/longbridge/gpui-kit", rev = "0c830f4d257e69fdd17200650533ab4ca9a40cc0" } # Fast over small. The opt-level reaches DuckDB's C++ too: libduckdb-sys # builds it with the `cc` crate, which passes Cargo's level on to the From 9433f410a7e366c706ff7a6b5cdbcf0ce79d4d41 Mon Sep 17 00:00:00 2001 From: JetSquirrel Date: Tue, 29 Sep 2026 21:35:01 +0800 Subject: [PATCH 3/3] Let dashboards draw pies and maps A .dash plot can now be a pie or a map, drawn by the same code as the results chart's. A pie takes x as its slices and y as their sizes, keeps every band (merging or dropping one would make the shares wrong) and folds past eight slices into "other". A map takes lat and lng in place of x and y, found by column name when left out, and an optional color column. check rejects the attributes a type has no use for (a map's x, a line's lat, a pie's series), and with --database verifies the new columns exist and that coordinates are numeric. Highlighting, completion, hover, check's JSON and the skill's reference know the new types and attributes. GeoData moves behind an Arc so a dashboard can prepare it off the UI thread. Co-Authored-By: Claude Opus 5.5 (1M context) --- skills/ducklocal/SKILL.md | 4 +- src/i18n.rs | 5 + src/spec/complete.rs | 13 ++- src/spec/highlight.rs | 2 +- src/spec/lsp.rs | 13 ++- src/spec/mod.rs | 3 + src/spec/model.rs | 211 ++++++++++++++++++++++++++++++++++++-- src/spec/prepare.rs | 177 +++++++++++++++++++++++++++++++- src/spec/view.rs | 50 +++++++-- src/ui/chart.rs | 103 +++++++++++++------ src/ui/geo.rs | 121 +++++++++++++++++----- tests/cli.rs | 41 ++++++++ tests/lsp.rs | 2 +- 13 files changed, 653 insertions(+), 92 deletions(-) diff --git a/skills/ducklocal/SKILL.md b/skills/ducklocal/SKILL.md index f863ee4..1515d1b 100644 --- a/skills/ducklocal/SKILL.md +++ b/skills/ducklocal/SKILL.md @@ -40,9 +40,9 @@ plot "revenue" { } ``` -A `query` block holds one `sql` attribute (one read-only statement — SELECT, WITH, FROM, VALUES, SHOW, DESCRIBE, SUMMARIZE or PIVOT — heredoc or string; DDL, DML, COPY, ATTACH and INSTALL are rejected). A `plot` block holds `type` (`line`, `bar`, `area`, `scatter`, `table`), `query` (a `query.name` reference), `x` and `y` (result columns, bare identifiers or quoted strings; `y` optional for `table`), optional `series` and `title`. No functions, conditionals, or interpolation exist. There is a working example at `examples/analysis_app/dashboard.dash`. +A `query` block holds one `sql` attribute (one read-only statement — SELECT, WITH, FROM, VALUES, SHOW, DESCRIBE, SUMMARIZE or PIVOT — heredoc or string; DDL, DML, COPY, ATTACH and INSTALL are rejected). A `plot` block holds `type` (`line`, `bar`, `area`, `scatter`, `pie`, `map`, `table`), `query` (a `query.name` reference), `x` and `y` (result columns, bare identifiers or quoted strings; `y` optional for `table`), optional `series` and `title`. A `pie` takes `x` as its slices and `y` as their sizes, and no `series`; it folds past eight slices into "other". A `map` takes `lat` and `lng` (degrees) instead of `x`/`y`/`series` — either may be omitted when a numeric column's name says it, such as `geo_lat` or `longitude` — plus an optional `color` column; for example `plot "stations" { type = "map" query = query.stations lat = geo_lat lng = geo_lng color = type }`. No functions, conditionals, or interpolation exist. There is a working example at `examples/analysis_app/dashboard.dash`. -Always validate before handing a spec over: `ducklocal check dashboard.dash`, or `ducklocal check dashboard.dash --database warehouse.duckdb` to also run every query read-only and verify every `x`/`y`/`series` against the columns the queries actually return (a non-numeric `y` is an error outside `table`). A spec mistake is exit 2 with kind `spec`, one `file:line: message` per diagnostic; a query that fails on the database is exit 1 with kind `sql`, one line per failing query — fix all of them, not just the first. Prefer the `--database` form whenever the database exists: without it nothing runs, so errors that appear only at execution (a cast the build cannot perform, a value that will not convert) go unseen. To see it rendered, open the file in the GUI (`ducklocal dashboard.dash` or drag it onto the window): it becomes a dashboard tab, a resizable vertical stack of the plots with per-plot inline errors. +Always validate before handing a spec over: `ducklocal check dashboard.dash`, or `ducklocal check dashboard.dash --database warehouse.duckdb` to also run every query read-only and verify every `x`/`y`/`series`/`lat`/`lng`/`color` against the columns the queries actually return (a non-numeric `y` is an error outside `table`, and so is a non-numeric `lat` or `lng`). A spec mistake is exit 2 with kind `spec`, one `file:line: message` per diagnostic; a query that fails on the database is exit 1 with kind `sql`, one line per failing query — fix all of them, not just the first. Prefer the `--database` form whenever the database exists: without it nothing runs, so errors that appear only at execution (a cast the build cannot perform, a value that will not convert) go unseen. To see it rendered, open the file in the GUI (`ducklocal dashboard.dash` or drag it onto the window): it becomes a dashboard tab, a resizable vertical stack of the plots with per-plot inline errors. ## Authoring an app diff --git a/src/i18n.rs b/src/i18n.rs index 3396d06..fdf1644 100644 --- a/src/i18n.rs +++ b/src/i18n.rs @@ -739,6 +739,11 @@ static STRINGS: &[(&str, &str, &str)] = &[ "查询结果中没有列 {}。可用的列：{}", "The query result has no column {}. Available columns: {}", ), + ( + "dashboard.map_no_coordinates", + "地图需要经纬度列：请用 lat 和 lng 指定。查询返回的列：{}", + "A map needs coordinate columns: name them with lat and lng. The query returns: {}", + ), ( "dashboard.empty", "这个 spec 没有声明任何 plot 块。", diff --git a/src/spec/complete.rs b/src/spec/complete.rs index 6fe084a..494921b 100644 --- a/src/spec/complete.rs +++ b/src/spec/complete.rs @@ -12,6 +12,11 @@ use super::model; +/// Every attribute a plot block may hold, in the order completion offers +/// them: the common ones first, a map's own after. +pub(crate) const PLOT_ATTRS: [&str; 9] = + ["type", "query", "x", "y", "series", "title", "lat", "lng", "color"]; + /// One thing that could be inserted at the cursor. #[derive(Debug, Clone, PartialEq)] pub(crate) struct Completion { @@ -95,7 +100,7 @@ pub(crate) fn complete(source: &str, line: usize, col: usize) -> Vec } match block_at(source, &lines, line, &before) { - Some(kind) if kind == "plot" => ["type", "query", "x", "y", "series", "title"] + Some(kind) if kind == "plot" => PLOT_ATTRS .into_iter() .map(|name| attribute(name, "plot")) .collect(), @@ -322,7 +327,7 @@ mod tests { let candidates = complete("plot \"p\" {\n \n}", 2, 3); assert_eq!( labels(&candidates), - ["type", "query", "x", "y", "series", "title"] + super::PLOT_ATTRS ); assert!(candidates .iter() @@ -337,7 +342,7 @@ mod tests { fn plot_types_after_type_equals() { // Inside an opened quote the insert text is the bare word. let candidates = complete("plot \"p\" {\n type = \"li\n}", 2, 12); - assert_eq!(labels(&candidates), ["line", "bar", "area", "scatter", "table"]); + assert_eq!(labels(&candidates), model::PLOT_TYPES); assert!(candidates .iter() .all(|c| c.kind == CompletionKind::Value)); @@ -377,7 +382,7 @@ mod tests { let source = "query \"q1\" {\n sql = \"SELECT 1\"\n\nplot \"p\" {\n t\n"; assert_eq!( labels(&complete(source, 5, 3)), - ["type", "query", "x", "y", "series", "title"] + super::PLOT_ATTRS ); } diff --git a/src/spec/highlight.rs b/src/spec/highlight.rs index 9e231fe..3f6b1b8 100644 --- a/src/spec/highlight.rs +++ b/src/spec/highlight.rs @@ -29,7 +29,7 @@ use gpui_kit::{Context, HighlightStyle, SharedString, Window}; pub const LANGUAGE: &str = "dash"; /// The plot types, coloured as constants: they are the language's own words. -const PLOT_TYPES: &[&str] = &["line", "bar", "area", "scatter", "table"]; +use super::model::PLOT_TYPES; /// What the scanner found: `.dash` tokens by highlight name, and the byte /// ranges of heredoc bodies, which are SQL. diff --git a/src/spec/lsp.rs b/src/spec/lsp.rs index 47f2c87..18a3192 100644 --- a/src/spec/lsp.rs +++ b/src/spec/lsp.rs @@ -407,7 +407,7 @@ fn hover_at(source: &str, position: Position) -> Option { Hit::Attr { name, .. } => attr_doc(&name)?.to_string(), Hit::Block { kind, name, .. } => match kind.as_str() { "query" => format!("**query \"{name}\"**\n\nA named query: one `sql` attribute holding one statement, as a heredoc or a string."), - "plot" => format!("**plot \"{name}\"**\n\nA named plot: `type`, `query`, `x` and `y`, plus optional `series` and `title`."), + "plot" => format!("**plot \"{name}\"**\n\nA named plot: `type`, `query`, `x` and `y`, plus optional `series` and `title`; a `map` takes `lat`, `lng` and `color` instead of `x`, `y` and `series`."), _ => return None, }, Hit::Ref { segments, .. } => { @@ -462,11 +462,14 @@ fn validate(source: &str) -> Option { fn attr_doc(name: &str) -> Option<&'static str> { Some(match name { "sql" => "**sql**\n\nThe query's one statement, as a heredoc or a string. A query block holds only this.", - "type" => "**type**\n\nThe plot's kind: one of `line`, `bar`, `area`, `scatter`, `table`.", + "type" => "**type**\n\nThe plot's kind: one of `line`, `bar`, `area`, `scatter`, `pie`, `map`, `table`.", "query" => "**query**\n\nThe query block this plot draws: `query = query.some_name`.", - "x" => "**x**\n\nA column of the query's result, as a bare identifier or a quoted string.", - "y" => "**y**\n\nThe numeric column the plot draws; optional when the type is `table`.", - "series" => "**series**\n\nOptional: the column that splits the plot into one line or bar group per value.", + "x" => "**x**\n\nA column of the query's result, as a bare identifier or a quoted string; a `pie`'s slices. Not for a `map`.", + "y" => "**y**\n\nThe numeric column the plot draws; a `pie`'s slice sizes. Optional for `table`, not for a `map`.", + "series" => "**series**\n\nOptional: the column that splits the plot into one line or bar group per value. Not for a `pie` or a `map`.", + "lat" => "**lat**\n\nA `map`'s latitude column, in degrees. Optional when a column's name says it (`lat`, `geo_lat`, `latitude`).", + "lng" => "**lng**\n\nA `map`'s longitude column, in degrees. Optional when a column's name says it (`lng`, `lon`, `longitude`).", + "color" => "**color**\n\nOptional, for a `map`: the column its points are colored by; the five most common values get a color, the rest share one.", "title" => "**title**\n\nOptional: the plot's title.", _ => return None, }) diff --git a/src/spec/mod.rs b/src/spec/mod.rs index 9791b0e..5e15f0e 100644 --- a/src/spec/mod.rs +++ b/src/spec/mod.rs @@ -168,6 +168,9 @@ pub fn check(args: &[OsString]) -> Result { "x": p.x, "y": p.y, "series": p.series, + "lat": p.lat, + "lng": p.lng, + "color": p.color, "title": p.title, "line": p.line, })).collect::>(), diff --git a/src/spec/model.rs b/src/spec/model.rs index db7a2de..894ffef 100644 --- a/src/spec/model.rs +++ b/src/spec/model.rs @@ -6,6 +6,10 @@ //! * a `query` block holds one `sql` attribute — one statement, nothing else; //! * a `plot` block holds `type`, `query`, `x`, and (unless a table) `y`, //! plus an optional `series` and `title`; +//! * a `pie` takes `x` (the slices) and `y` (their sizes), and no `series`; +//! * a `map` takes `lat` and `lng` instead of `x` and `y` — either may be left +//! out when a column's name says what it is (`geo_lat`, `longitude`) — and +//! an optional `color`; //! * `query = query.latency` names a query block that exists; //! * `x`, `y`, `series` name result columns — as bare identifiers when the //! column allows it, as strings when it does not (`"Revenue (USD)"`). @@ -19,8 +23,12 @@ use std::collections::HashSet; use super::syntax::{self, Attr, Block, File, RefSite, Value}; /// The plot types the format knows. Each is a promise the renderer can keep: -/// x/y for the continuous ones, anything tabular for `table`. -pub(crate) const PLOT_TYPES: &[&str] = &["line", "bar", "area", "scatter", "table"]; +/// x/y for the continuous ones and `pie`, anything tabular for `table`, +/// coordinates for `map`. +pub(crate) const PLOT_TYPES: &[&str] = &["line", "bar", "area", "scatter", "pie", "map", "table"]; + +/// The attributes only a `map` takes. +const MAP_ATTRS: &[&str] = &["lat", "lng", "color"]; #[derive(Debug, Clone)] pub(crate) struct Spec { @@ -55,6 +63,11 @@ pub(crate) struct Plot { pub y: Option, pub series: Option, pub title: Option, + /// A `map`'s coordinate columns; `None` finds them by name. + pub lat: Option, + pub lng: Option, + /// The column a `map` colors its points by; `None` picks one. + pub color: Option, /// Line and column of the block's kind keyword. pub line: usize, pub col: usize, @@ -358,6 +371,9 @@ fn plot(block: &Block, diagnostics: &mut Vec) -> Option { let mut y = None; let mut series = None; let mut title = None; + let mut lat = None; + let mut lng = None; + let mut color = None; let mut seen: HashSet<&str> = HashSet::new(); for attr in &block.attrs { if !seen.insert(attr.name.as_str()) { @@ -394,15 +410,40 @@ fn plot(block: &Block, diagnostics: &mut Vec) -> Option { "y" => y = column(attr, diagnostics), "series" => series = column(attr, diagnostics), "title" => title = string(attr, diagnostics), + "lat" => lat = column(attr, diagnostics), + "lng" => lng = column(attr, diagnostics), + "color" => color = column(attr, diagnostics), other => diagnostics.push(Diagnostic::at_attr( attr, format!( - "A plot block holds type, query, x, y, series, title; unknown attribute: {other}" + "A plot block holds type, query, x, y, series, title, and for a map lat, lng, color; unknown attribute: {other}" ), )), } } - for name in ["type", "query", "x"] { + let kind_name = kind.clone().unwrap_or_default(); + let is_map = kind_name == "map"; + // Attributes the type has no use for are mistakes, not no-ops: a map's + // `x` or a line's `lat` says the author expects something that will not + // happen. + for attr in &block.attrs { + let name = attr.name.as_str(); + let misplaced = if is_map { + matches!(name, "x" | "y" | "series") + .then(|| format!("A map plot places points by lat and lng; it takes no {name}")) + } else if !kind_name.is_empty() && MAP_ATTRS.contains(&name) { + Some(format!("{name} is for map plots; this plot is a {kind_name}")) + } else if kind_name == "pie" && name == "series" { + Some("A pie draws one series; it takes no series".to_string()) + } else { + None + }; + if let Some(message) = misplaced { + diagnostics.push(Diagnostic::at_attr(attr, message)); + } + } + let required: &[&str] = if is_map { &["type", "query"] } else { &["type", "query", "x"] }; + for &name in required { if !seen.contains(name) { diagnostics.push(Diagnostic::at_block( block, @@ -414,7 +455,7 @@ fn plot(block: &Block, diagnostics: &mut Vec) -> Option { // is only for attributes never written. y is required once the type is // known to need one. let kind = kind.unwrap_or_default(); - if !kind.is_empty() && kind != "table" && y.is_none() { + if !kind.is_empty() && kind != "table" && kind != "map" && y.is_none() { diagnostics.push(Diagnostic::at_block( block, format!("plot block {:?} requires y", block.name), @@ -428,6 +469,9 @@ fn plot(block: &Block, diagnostics: &mut Vec) -> Option { y, series, title, + lat, + lng, + color, line: block.line, col: block.col, span: block.kind_span, @@ -500,10 +544,47 @@ pub(crate) fn check_columns(spec: &Spec, columns: &ColumnLookup) -> Vec>() + .join(", ") + }; + // A map may leave its coordinates to the column names; if the names + // do not say, the plot has nothing to place. + let (mut lat, mut lng) = (plot.lat.clone(), plot.lng.clone()); + if plot.kind == "map" && (lat.is_none() || lng.is_none()) { + let (guess_lat, guess_lng) = crate::ui::geo::guess_coordinate_columns( + available.iter().map(|(name, _)| name.as_str()), + ); + let guessed = |ix: Option| ix.map(|ix| available[ix].0.clone()); + lat = lat.or_else(|| guessed(guess_lat)); + lng = lng.or_else(|| guessed(guess_lng)); + for (name, found) in [("lat", &lat), ("lng", &lng)] { + if found.is_none() { + diagnostics.push(Diagnostic { + line: plot.line, + col: plot.col, + span: plot.span, + message: format!( + "map plot {:?} names no {name}, and no column of query {:?} looks like one; set {name} to one of: {}", + plot.name, + query.name, + returns() + ), + }); + } + } + } + let x = (plot.kind != "map").then_some(plot.x.as_str()); for (name, column) in [ - ("x", Some(plot.x.as_str())), + ("x", x), ("y", plot.y.as_deref()), ("series", plot.series.as_deref()), + ("lat", plot.lat.as_deref()), + ("lng", plot.lng.as_deref()), + ("color", plot.color.as_deref()), ] .into_iter() .filter_map(|(name, column)| column.map(|c| (name, c))) @@ -526,6 +607,26 @@ pub(crate) fn check_columns(spec: &Spec, columns: &ColumnLookup) -> Vec = diagnostics.iter().map(|d| d.message.as_str()).collect(); assert!(messages.iter().any(|m| m.contains("Duplicate query")), "{messages:?}"); - assert!(messages.iter().any(|m| m.contains("\"pie\"")), "{messages:?}"); + assert!(messages.iter().any(|m| m.contains("\"donut\"")), "{messages:?}"); assert!(messages.iter().any(|m| m.contains("does not define")), "{messages:?}"); assert!(messages.iter().any(|m| m.contains("requires x")), "{messages:?}"); assert!(messages.iter().any(|m| m.contains("requires y")), "{messages:?}"); @@ -669,7 +770,7 @@ wat "huh" {} let pie = diagnostics .iter() - .find(|d| d.message.contains("\"pie\"")) + .find(|d| d.message.contains("\"donut\"")) .unwrap(); assert_eq!((pie.line, pie.col), (5, 3)); assert_eq!(&source[pie.span.0..pie.span.1], "type"); @@ -820,4 +921,96 @@ plot "p" { type = "line" query = query.q x = "Order Date" y = revenue } ); assert_eq!(spec.plots[0].x, "Order Date"); } + + #[test] + fn pies_and_maps_validate() { + let spec = validate_ok( + r#" +query "q" { sql = "SELECT 1" } +plot "share" { type = "pie" query = query.q x = channel y = total } +plot "where" { + type = "map" + query = query.q + lat = geo_lat + lng = geo_lng + color = type +} +plot "guessed" { type = "map" query = query.q } +"#, + ); + assert_eq!(spec.plots[0].kind, "pie"); + let map = &spec.plots[1]; + assert_eq!(map.lat.as_deref(), Some("geo_lat")); + assert_eq!(map.lng.as_deref(), Some("geo_lng")); + assert_eq!(map.color.as_deref(), Some("type")); + // A map needs neither x nor y, and may leave its coordinates to the + // column names. + assert_eq!(spec.plots[2].lat, None); + } + + #[test] + fn attributes_a_type_has_no_use_for_are_mistakes() { + let diagnostics = validate_err( + r#" +query "q" { sql = "SELECT 1" } +plot "a" { type = "map" query = query.q x = lng y = lat } +plot "b" { type = "line" query = query.q x = t y = v lat = geo_lat } +plot "c" { type = "pie" query = query.q x = t y = v series = s } +"#, + ); + let messages: Vec<&str> = diagnostics.iter().map(|d| d.message.as_str()).collect(); + assert!(messages.iter().any(|m| m.contains("takes no x")), "{messages:?}"); + assert!(messages.iter().any(|m| m.contains("takes no y")), "{messages:?}"); + assert!( + messages.iter().any(|m| m.contains("lat is for map plots")), + "{messages:?}" + ); + assert!( + messages.iter().any(|m| m.contains("takes no series")), + "{messages:?}" + ); + // Each points at the attribute, on its own line. + let lat = diagnostics + .iter() + .find(|d| d.message.contains("lat is for map plots")) + .unwrap(); + assert_eq!(lat.line, 4); + } + + #[test] + fn a_map_checks_its_coordinates_against_the_query() { + let spec = validate_ok( + r#" +query "q" { sql = "SELECT 1" } +plot "named" { type = "map" query = query.q lat = geo_lat lng = code } +plot "guessed" { type = "map" query = query.q } +"#, + ); + let stations = |_: &str| -> Result, String> { + Ok(vec![ + ("code".into(), "VARCHAR".into()), + ("geo_lat".into(), "DOUBLE".into()), + ("geo_lng".into(), "DOUBLE".into()), + ]) + }; + let diagnostics = check_columns(&spec, &stations); + // A text longitude places nothing; the guessed map finds both. + assert_eq!(diagnostics.len(), 1, "{diagnostics:?}"); + assert!(diagnostics[0].message.contains("lng = \"code\""), "{diagnostics:?}"); + + let no_coordinates = |_: &str| -> Result, String> { + Ok(vec![("code".into(), "VARCHAR".into())]) + }; + let diagnostics = check_columns(&spec, &no_coordinates); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("names no lat") && d.message.contains("guessed")), + "{diagnostics:?}" + ); + assert!( + diagnostics.iter().any(|d| d.message.contains("\"geo_lat\"")), + "a named column the query lacks is reported: {diagnostics:?}" + ); + } } diff --git a/src/spec/prepare.rs b/src/spec/prepare.rs index 56cf618..c3aca21 100644 --- a/src/spec/prepare.rs +++ b/src/spec/prepare.rs @@ -7,6 +7,11 @@ //! and a half, bars truncated past two hundred — because a chart that wedges //! the renderer is worse than a chart that says it was merged. //! +//! A `pie` is the same matrix with one series, never merged or truncated — +//! its slices must add up to the whole — then folded into its largest +//! slices. A `map` skips the matrix: it is the results chart's `GeoData`, +//! over the columns the spec names. +//! //! Everything here is pure and tested as such; the view (`view.rs`) only //! renders what this prepares. @@ -18,7 +23,9 @@ use gpui_kit::SharedString; use super::model::Plot; use crate::i18n::trf; use crate::query::QueryResult; -use crate::ui::chart::{format_value, parse_number}; +use crate::query::ColumnKind; +use crate::ui::chart::{format_value, parse_number, pie_slices, PieSlice}; +use crate::ui::geo::{is_lat_name, is_lng_name, GeoData}; /// A chart is at most a couple of thousand pixels wide; past this, points are /// bucket-averaged, as in the results chart. @@ -55,6 +62,10 @@ pub struct PreparedPlot { pub label_name: String, pub series_names: Vec, pub points: Vec>, + /// A `pie`'s slices, largest first. + pub pie: Vec>, + /// A `map`'s points. + pub geo: Option>, /// What was dropped or merged to keep the plot drawable. pub notice: Option, /// Why there is nothing to draw: the query failed, or a column the plot @@ -75,6 +86,8 @@ pub(crate) fn prepare(plot: &Plot, result: Option<&Result>) label_name: plot.x.clone(), series_names: Vec::new(), points: Vec::new(), + pie: Vec::new(), + geo: None, notice: None, failure, }; @@ -95,6 +108,46 @@ pub(crate) fn prepare(plot: &Plot, result: Option<&Result>) .iter() .position(|column| column.name.eq_ignore_ascii_case(name)) }; + if plot.kind == "map" { + // Named columns are taken at their word; a missing one is found by + // name among the numeric columns, as the results chart finds them. + let find = |named: &Option, looks_like: fn(&str) -> bool, skip: Option| { + match named { + Some(name) => column(name).ok_or_else(|| missing_column(name, result)), + None => result + .columns + .iter() + .enumerate() + .find(|(ix, c)| { + Some(*ix) != skip && c.kind == ColumnKind::Numeric && looks_like(&c.name) + }) + .map(|(ix, _)| ix) + .ok_or_else(|| missing_coordinates(result)), + } + }; + let lat_ix = match find(&plot.lat, is_lat_name, None) { + Ok(ix) => ix, + Err(message) => return base(Some(message)), + }; + let lng_ix = match find(&plot.lng, is_lng_name, Some(lat_ix)) { + Ok(ix) => ix, + Err(message) => return base(Some(message)), + }; + let color_ix = match plot.color.as_deref().map(|name| column(name).ok_or(name)) { + Some(Ok(ix)) => Some(ix), + Some(Err(name)) => return base(Some(missing_column(name, result))), + None => None, + }; + let geo = GeoData::from_columns(result, lat_ix, lng_ix, color_ix); + return PreparedPlot { + label_name: format!( + "{} / {}", + result.columns[lat_ix].name, result.columns[lng_ix].name + ), + geo: Some(Arc::new(geo)), + ..base(None) + }; + } let Some(x_ix) = column(&plot.x) else { return base(Some(missing_column(&plot.x, result))); }; @@ -214,7 +267,10 @@ pub(crate) fn prepare(plot: &Plot, result: Option<&Result>) } let source_points = points.len(); - if plot.kind == "bar" { + if plot.kind == "pie" { + // Every band stays: the slices fold below, and a merged or dropped + // band would make the shares wrong. + } else if plot.kind == "bar" { if source_points > MAX_BARS { points.truncate(MAX_BARS); notices.push(trf( @@ -276,6 +332,16 @@ pub(crate) fn prepare(plot: &Plot, result: Option<&Result>) point.label = SharedString::from(format_value(first)); } + let points: Vec> = points.into_iter().map(Arc::new).collect(); + let (pie, pie_skipped) = if plot.kind == "pie" { + pie_slices(&points) + } else { + (Vec::new(), 0) + }; + if pie_skipped > 0 { + notices.push(trf("chart.pie.notice.skipped", &[&pie_skipped.to_string()])); + } + PreparedPlot { name: plot.name.clone(), title: plot.title.clone().unwrap_or_else(|| plot.name.clone()), @@ -283,12 +349,25 @@ pub(crate) fn prepare(plot: &Plot, result: Option<&Result>) query: plot.query.clone(), label_name: result.columns[x_ix].name.clone(), series_names, - points: points.into_iter().map(Arc::new).collect(), + points, + pie, + geo: None, notice: (!notices.is_empty()).then(|| notices.join(" · ")), failure: None, } } +/// Why a `map` with no `lat` or `lng` has nothing to place. +fn missing_coordinates(result: &QueryResult) -> String { + let available = result + .columns + .iter() + .map(|column| column.name.as_str()) + .collect::>() + .join(", "); + trf("dashboard.map_no_coordinates", &[&available]) +} + fn missing_column(name: &str, result: &QueryResult) -> String { let available = result .columns @@ -313,6 +392,9 @@ mod tests { y: y.map(str::to_string), series: series.map(str::to_string), title: None, + lat: None, + lng: None, + color: None, line: 1, col: 1, span: (0, 4), @@ -462,4 +544,93 @@ mod tests { assert!(line.points.iter().all(|p| p.values[0] == 1.0)); assert!(line.notice.is_some()); } + + fn map_plot(lat: Option<&str>, lng: Option<&str>, color: Option<&str>) -> Plot { + Plot { + kind: "map".into(), + x: String::new(), + y: None, + lat: lat.map(str::to_string), + lng: lng.map(str::to_string), + color: color.map(str::to_string), + ..plot("map", None, None) + } + } + + fn stations() -> QueryResult { + QueryResult { + columns: vec![ + column("name", ColumnKind::Text), + column("type", ColumnKind::Text), + column("geo_lat", ColumnKind::Numeric), + column("geo_lng", ColumnKind::Numeric), + ], + rows: [ + ["Utrecht", "mega", "52.09", "5.11"], + ["Zwolle", "ic", "52.50", "6.09"], + ["Aalten", "stop", "NULL", "6.57"], + ] + .iter() + .map(|r| r.iter().map(|c| c.to_string()).collect()) + .collect(), + elapsed_ms: 0, + truncated: false, + } + } + + #[test] + fn a_map_places_the_rows_its_columns_locate() { + let result = Ok(stations()); + let named = prepare( + &map_plot(Some("geo_lat"), Some("geo_lng"), Some("type")), + Some(&result), + ); + assert_eq!(named.failure, None); + let geo = named.geo.expect("a map carries its points"); + assert_eq!(geo.point_count(), 2); + assert_eq!(geo.dropped, 1, "a NULL latitude places nothing"); + assert_eq!(geo.category_name.as_deref(), Some("type")); + + // Left out, the coordinates are found by name. + let guessed = prepare(&map_plot(None, None, None), Some(&result)); + assert_eq!(guessed.geo.map(|g| g.point_count()), Some(2)); + assert_eq!(guessed.label_name, "geo_lat / geo_lng"); + } + + #[test] + fn a_map_without_coordinates_says_so() { + let result = Ok(QueryResult { + columns: vec![column("name", ColumnKind::Text)], + rows: vec![vec!["x".into()]], + elapsed_ms: 0, + truncated: false, + }); + let prepared = prepare(&map_plot(None, None, None), Some(&result)); + assert!(prepared.geo.is_none()); + assert!(prepared.failure.is_some()); + + let prepared = prepare(&map_plot(Some("nope"), Some("geo_lng"), None), Some(&Ok(stations()))); + assert!(prepared.failure.unwrap().contains("nope")); + } + + #[test] + fn a_pie_keeps_every_band_and_folds_the_small_ones() { + // More bands than a bar chart keeps: a pie must add up to the whole. + let rows: Vec> = (1..=300) + .map(|ix| vec![format!("c{ix}"), ix.to_string()]) + .chain([vec!["neg".to_string(), "-1".to_string()]]) + .collect(); + let result = Ok(QueryResult { + columns: vec![column("x", ColumnKind::Text), column("y", ColumnKind::Numeric)], + rows, + elapsed_ms: 0, + truncated: false, + }); + let pie = prepare(&plot("pie", Some("y"), None), Some(&result)); + assert_eq!(pie.points.len(), 301, "nothing truncated"); + assert_eq!(pie.pie.len(), 8, "folded to the largest slices"); + let total: f64 = pie.pie.iter().map(|s| s.value() as f64).sum(); + assert_eq!(total, (1..=300).sum::() as f64); + assert!(pie.notice.unwrap().contains('1'), "the negative band is reported"); + } } diff --git a/src/spec/view.rs b/src/spec/view.rs index 2fd635e..257fd5c 100644 --- a/src/spec/view.rs +++ b/src/spec/view.rs @@ -56,7 +56,8 @@ use crate::query::{ColumnKind, QueryOutcome, QueryResult}; use crate::spec::complete::{self, CompletionKind}; use crate::spec::model::{self, Spec}; use crate::spec::plot::{GroupedBars, SeriesPlot, x_label_count}; -use crate::ui::chart::format_value; +use crate::ui::chart::{format_value, legend_row, map_notes, pie_parts}; +use crate::ui::geo::GeoPlot; use crate::spec::prepare::{prepare, PlotPoint, PreparedPlot}; use crate::ui::completion::starts_with_ignore_case; use crate::ui::results::fit_column_width; @@ -302,24 +303,47 @@ impl Dashboard { window: &mut Window, cx: &mut Context, ) -> AnyElement { - let (title, notice, failure, is_table, is_empty) = { + let (title, mut notice, failure, is_table, is_empty) = { let plot = &self.plots[ix]; + let is_empty = match plot.kind.as_str() { + "map" => plot.geo.as_ref().is_none_or(|geo| geo.point_count() == 0), + "pie" => plot.pie.is_empty(), + _ => plot.points.is_empty(), + }; ( plot.title.clone(), plot.notice.clone(), plot.failure.clone(), plot.kind == "table", - plot.points.is_empty(), + is_empty, ) }; // The axis subtitle the results chart shows: which series, over which - // x column. Tables and failures have nothing to say. - let axes = (!is_table && failure.is_none() && !is_empty).then(|| { + // x column — for a pie, what the shares are of; for a map, which + // columns place the points. Tables and failures have nothing to say. + let drawn = !is_table && failure.is_none() && !is_empty; + let mut legend = Vec::new(); + let axes = drawn.then(|| { let plot = &self.plots[ix]; - trf( - "chart.title.by", - &[&plot.series_names.join(", "), &plot.label_name], - ) + let series = plot.series_names.join(", "); + match plot.kind.as_str() { + "map" => { + if let Some(geo) = &plot.geo { + let (notes, key) = map_notes(geo, cx); + notice = match (notice.take(), notes) { + (Some(a), Some(b)) => Some(format!("{a} · {b}")), + (a, b) => a.or(b), + }; + legend = key; + } + plot.label_name.clone() + } + "pie" => { + legend = pie_parts(&plot.pie, series.clone().into(), "dashboard-pie-key", cx).1; + trf("chart.pie.title", &[&series, &plot.label_name]) + } + _ => trf("chart.title.by", &[&series, &plot.label_name]), + } }); let body = if let Some(message) = failure { @@ -380,7 +404,8 @@ impl Dashboard { .text_color(cx.theme().muted_foreground) .child(axes), ) - }), + }) + .when(!legend.is_empty(), |this| this.child(legend_row(legend, cx))), ) .child(div().flex_1().min_h_0().child(body)) .into_any_element() @@ -773,6 +798,11 @@ fn chart_element(plot_ix: usize, plot: &PreparedPlot, cx: &App) -> AnyElement { .unwrap_or_else(|| plot.title.clone()); match plot.kind.as_str() { + "pie" => pie_parts(&plot.pie, name.into(), named_id(), cx).0, + "map" => match &plot.geo { + Some(geo) => GeoPlot::new(named_id(), geo.clone()).into_any_element(), + None => empty_chart(cx), + }, "bar" if single => BarChart::new(plot.points.clone()) .band(|d: &Arc| d.band.clone()) .value(|d: &Arc| d.values[0]) diff --git a/src/ui/chart.rs b/src/ui/chart.rs index d7eac6d..b7a550c 100644 --- a/src/ui/chart.rs +++ b/src/ui/chart.rs @@ -54,6 +54,7 @@ impl ChartKind { } /// One pie slice: a band's first-series value, or the fold of the smallest. +#[derive(Debug)] pub struct PieSlice { name: SharedString, value: f32, @@ -61,6 +62,13 @@ pub struct PieSlice { slot: Option, } +impl PieSlice { + #[cfg(test)] + pub(crate) fn value(&self) -> f32 { + self.value + } +} + /// A chart is at most a couple of thousand pixels wide, so plotting more /// points than this cannot show more detail — it only multiplies the /// primitives the renderer has to push. A wide `PIVOT` over a fortnight of @@ -88,11 +96,11 @@ pub struct ChartData { is_time_series: bool, rows: Vec>, /// The first series as pie slices, largest first. - pie: Vec>, + pie: Vec>, /// Rows the pie leaves out: a zero, negative or missing value is no slice. pie_skipped: usize, /// Set when the result has a latitude and a longitude column. - geo: Option>, + geo: Option>, /// `name (type)` per column, for the "no numeric column" message. Only /// populated when there is nothing to plot. detected: Vec, @@ -104,7 +112,7 @@ pub struct ChartData { impl ChartData { pub fn prepare(result: &QueryResult) -> Self { - let geo = GeoData::detect(result).map(Rc::new); + let geo = GeoData::detect(result).map(Arc::new); let mut value_ixes: Vec = result .columns .iter() @@ -269,7 +277,7 @@ impl ChartData { /// The first series as pie slices, largest first, the smallest folded into /// "other" past `MAX_SLICES`; plus how many rows had no positive value. -fn pie_slices(rows: &[Arc]) -> (Vec>, usize) { +pub(crate) fn pie_slices(rows: &[Arc]) -> (Vec>, usize) { let mut positive: Vec<(SharedString, f64)> = rows .iter() .filter(|r| r.values[0] > 0.) @@ -284,12 +292,12 @@ fn pie_slices(rows: &[Arc]) -> (Vec>, usize) { positive.len() }; let rest: f64 = positive[kept..].iter().map(|(_, v)| v).sum(); - let mut slices: Vec> = positive + let mut slices: Vec> = positive .into_iter() .take(kept) .enumerate() .map(|(slot, (name, value))| { - Rc::new(PieSlice { + Arc::new(PieSlice { name, value: value as f32, slot: Some(slot), @@ -297,7 +305,7 @@ fn pie_slices(rows: &[Arc]) -> (Vec>, usize) { }) .collect(); if folded { - slices.push(Rc::new(PieSlice { + slices.push(Arc::new(PieSlice { name: tr("chart.map.other").into(), value: rest as f32, slot: None, @@ -526,23 +534,15 @@ fn chart_frame( .when_some(notice, |this, notice| { this.child(div().text_xs().text_color(muted).child(notice)) }) - .when(!legend.is_empty(), |this| { - this.child(h_flex().flex_wrap().gap_x_3().gap_y_1().children( - legend.into_iter().map(|(color, name)| { - h_flex() - .gap_1() - .items_center() - .child(div().size_2().rounded_full().bg(color)) - .child(div().text_xs().text_color(muted).child(name)) - }), - )) - }), + .when(!legend.is_empty(), |this| this.child(legend_row(legend, cx))), ) .child(div().flex_1().min_h_0().child(chart)) .into_any_element() } -fn render_map(geo: Rc, cx: &App) -> AnyElement { +/// What a map says under its title: capping, dropped rows, the color +/// column; and its legend. Shared with dashboards. +pub(crate) fn map_notes(geo: &GeoData, cx: &App) -> (Option, Vec<(Hsla, SharedString)>) { let mut notices = Vec::new(); if let Some(total) = geo.capped_from { notices.push(trf( @@ -562,20 +562,47 @@ fn render_map(geo: Rc, cx: &App) -> AnyElement { .enumerate() .map(|(slot, name)| (geo.category_color(slot, cx), name.clone())) .collect(); + ((!notices.is_empty()).then(|| notices.join(" · ")), legend) +} + +fn render_map(geo: Arc, cx: &App) -> AnyElement { + let (notice, legend) = map_notes(&geo, cx); chart_frame( trf("chart.map.title", &[&geo.lat_name, &geo.lng_name]), trf("chart.title.point_count", &[&geo.point_count().to_string()]), - (!notices.is_empty()).then(|| notices.join(" · ")), + notice, legend, GeoPlot::new("results-chart-map", geo).into_any_element(), cx, ) } -fn render_pie(data: &ChartData, cx: &App) -> AnyElement { - if data.pie.is_empty() { - return empty_chart_state(tr("chart.pie.no_positive"), &[], cx); - } +/// A key of colored dots and names, wrapping as wide as it is given. +pub(crate) fn legend_row(legend: Vec<(Hsla, SharedString)>, cx: &App) -> AnyElement { + let muted = cx.theme().muted_foreground; + h_flex() + .flex_wrap() + .gap_x_3() + .gap_y_1() + .children(legend.into_iter().map(|(color, name)| { + h_flex() + .gap_1() + .items_center() + .child(div().size_2().rounded_full().bg(color)) + .child(div().text_xs().text_color(muted).child(name)) + })) + .into_any_element() +} + +/// A pie over `slices` and its legend (each slice with its share), shared by +/// the results panel and dashboards so both color and fold slices alike. +/// Also returns the total the shares are of. +pub(crate) fn pie_parts( + slices: &[Arc], + series: SharedString, + id: impl Into, + cx: &App, +) -> (AnyElement, Vec<(Hsla, SharedString)>, f64) { let palette = [ cx.theme().chart_1, cx.theme().chart_2, @@ -591,9 +618,8 @@ fn render_pie(data: &ChartData, cx: &App) -> AnyElement { Some(slot) => palette[slot % palette.len()].opacity(0.55), None => muted.opacity(0.6), }; - let total: f64 = data.pie.iter().map(|s| s.value as f64).sum(); - let legend = data - .pie + let total: f64 = slices.iter().map(|s| s.value as f64).sum(); + let legend = slices .iter() .map(|s| { let share = s.value as f64 / total * 100.; @@ -603,15 +629,24 @@ fn render_pie(data: &ChartData, cx: &App) -> AnyElement { ) }) .collect(); - let series = data.series_names[0].clone(); - let chart = PieChart::new(data.pie.clone()) - .value(|s: &Rc| s.value) - .color(move |s: &Rc| color(s)) + let chart = PieChart::new(slices.to_vec()) + .value(|s: &Arc| s.value) + .color(move |s: &Arc| color(s)) .pad_angle(0.01) - .tooltip_name(|s: &Rc| s.name.clone()) - .name(series.clone()) - .id("results-chart-pie") + .tooltip_name(|s: &Arc| s.name.clone()) + .name(series) + .id(id) .into_any_element(); + (chart, legend, total) +} + +fn render_pie(data: &ChartData, cx: &App) -> AnyElement { + if data.pie.is_empty() { + return empty_chart_state(tr("chart.pie.no_positive"), &[], cx); + } + let series = data.series_names[0].clone(); + let (chart, legend, total) = + pie_parts(&data.pie, series.clone().into(), "results-chart-pie", cx); let notice = (data.pie_skipped > 0) .then(|| trf("chart.pie.notice.skipped", &[&data.pie_skipped.to_string()])); let notice = match (data.notice.clone(), notice) { diff --git a/src/ui/geo.rs b/src/ui/geo.rs index 60649ba..80eed8e 100644 --- a/src/ui/geo.rs +++ b/src/ui/geo.rs @@ -14,7 +14,7 @@ use std::collections::HashMap; use std::f64::consts::FRAC_PI_4; -use std::rc::Rc; +use std::sync::Arc; use gpui_kit::component::plot::label::{Text, TEXT_SIZE}; use gpui_kit::component::plot::tooltip::{Dot, Tooltip, TooltipState}; @@ -53,6 +53,7 @@ const EDGE_PAD: f32 = 8.; /// leave a wide plot with three meridians across it. const LINE_SPACING: f64 = 90.; +#[derive(Debug)] pub(crate) struct GeoPoint { lat: f64, lng: f64, @@ -65,6 +66,7 @@ pub(crate) struct GeoPoint { } /// Everything the map needs, derived from a result set once. +#[derive(Debug)] pub struct GeoData { pub lat_name: String, pub lng_name: String, @@ -74,7 +76,7 @@ pub struct GeoData { /// one is "other", drawn in the muted color. pub categories: Vec, pub folded_other: bool, - points: Rc>, + points: Arc>, /// Projected extent: `(min_x, max_x, min_y, max_y)`. extent: (f64, f64, f64, f64), /// Rows whose coordinates were missing or out of range. @@ -95,27 +97,44 @@ impl GeoData { let lng_ix = (0..result.columns.len()) .filter(numeric) .find(|&ix| ix != lat_ix && is_lng_name(&result.columns[ix].name))?; - - let valid: Vec<(usize, f64, f64)> = result - .rows - .iter() - .enumerate() - .filter_map(|(ix, row)| { - let lat = parse_number(row.get(lat_ix)?)?; - let lng = parse_number(row.get(lng_ix)?)?; - ((-90.0..=90.0).contains(&lat) && (-180.0..=180.0).contains(&lng)) - .then_some((ix, lat, lng)) - }) - .collect(); + let valid = valid_coordinates(result, lat_ix, lng_ix); if valid.is_empty() || valid.len() * 2 < result.rows.len() { return None; } + Some(Self::build(result, lat_ix, lng_ix, valid, None)) + } + + /// The map a dashboard's `map` plot names outright: its latitude and + /// longitude columns, and optionally the column to color by (`None` picks + /// one as `detect` does). Rows without valid coordinates are dropped and + /// counted; nothing is second-guessed, so an empty map says the columns + /// held no coordinates rather than falling back to something else. + pub fn from_columns( + result: &QueryResult, + lat_ix: usize, + lng_ix: usize, + color_ix: Option, + ) -> Self { + let valid = valid_coordinates(result, lat_ix, lng_ix); + Self::build(result, lat_ix, lng_ix, valid, color_ix) + } + + fn build( + result: &QueryResult, + lat_ix: usize, + lng_ix: usize, + valid: Vec<(usize, f64, f64)>, + color_ix: Option, + ) -> Self { let dropped = result.rows.len() - valid.len(); let valid_count = valid.len(); let capped_from = (valid_count > MAX_GEO_POINTS).then_some(valid_count); let label_ix = label_column(result, &[lat_ix, lng_ix]); - let category = category_column(result, &valid, label_ix); + let category = match color_ix { + Some(column) => category_of(result, &valid, column), + None => category_column(result, &valid, label_ix), + }; let points: Vec = valid .into_iter() @@ -159,18 +178,18 @@ impl GeoData { None => (None, Vec::new(), false), }; - Some(Self { + Self { lat_name: result.columns[lat_ix].name.clone(), lng_name: result.columns[lng_ix].name.clone(), label_name: label_ix.map(|ix| result.columns[ix].name.clone()), category_name, categories, folded_other, - points: Rc::new(points), + points: Arc::new(points), extent, dropped, capped_from, - }) + } } pub fn point_count(&self) -> usize { @@ -188,6 +207,36 @@ impl GeoData { } } +/// `(row index, latitude, longitude)` for every row whose two cells parse as +/// coordinates in range. +fn valid_coordinates(result: &QueryResult, lat_ix: usize, lng_ix: usize) -> Vec<(usize, f64, f64)> { + result + .rows + .iter() + .enumerate() + .filter_map(|(ix, row)| { + let lat = parse_number(row.get(lat_ix)?)?; + let lng = parse_number(row.get(lng_ix)?)?; + ((-90.0..=90.0).contains(&lat) && (-180.0..=180.0).contains(&lng)) + .then_some((ix, lat, lng)) + }) + .collect() +} + +/// The latitude and longitude columns names alone suggest, as `detect` finds +/// them but without the values to check: what `ducklocal check` can say about +/// a `map` plot that leaves `lat` or `lng` out. +pub(crate) fn guess_coordinate_columns<'a>( + names: impl Iterator + Clone, +) -> (Option, Option) { + let lat = names.clone().position(is_lat_name); + let lng = names + .enumerate() + .find(|(ix, name)| Some(*ix) != lat && is_lng_name(name)) + .map(|(ix, _)| ix); + (lat, lng) +} + fn category_color(slot: usize, is_other: bool, cx: &App) -> Hsla { let theme = cx.theme(); if is_other { @@ -330,6 +379,32 @@ fn category_column( } let (column, sorted) = best?; + Some(fold_category(column, sorted)) +} + +/// Color by `column`, as a `color` attribute asks: every value it holds, the +/// most common first, the rest folded into "other" past the palette. +fn category_of(result: &QueryResult, valid: &[(usize, f64, f64)], column: usize) -> Option { + let mut counts: HashMap<&str, usize> = HashMap::new(); + for &(row_ix, _, _) in valid { + if let Some(value) = result.rows[row_ix].get(column) { + *counts.entry(value.as_str()).or_default() += 1; + } + } + if counts.is_empty() { + return None; + } + let mut sorted: Vec<(String, usize)> = counts + .into_iter() + .map(|(k, v)| (k.to_string(), v)) + .collect(); + sorted.sort_by(|a, b| b.1.cmp(&a.1).then_with(|| a.0.cmp(&b.0))); + Some(fold_category(column, sorted)) +} + +/// Legend slots for `sorted` values (most common first): one each up to the +/// palette's size, or all but the last slot plus "other" past it. +fn fold_category(column: usize, sorted: Vec<(String, usize)>) -> Category { let folded = sorted.len() > MAX_CATEGORIES; let kept = if folded { MAX_CATEGORIES - 1 @@ -350,12 +425,12 @@ fn category_column( if folded { legend.push(tr("chart.map.other").into()); } - Some(Category { + Category { column, slots, legend, folded, - }) + } } fn mercator_y(lat: f64) -> f64 { @@ -459,14 +534,14 @@ impl Viewport { } /// The map element. Rebuilt every frame from the cached `GeoData`; holding the -/// points behind an `Rc` keeps that a refcount bump. +/// points behind an `Arc` keeps that a refcount bump. pub(crate) struct GeoPlot { id: ElementId, - data: Rc, + data: Arc, } impl GeoPlot { - pub(crate) fn new(id: impl Into, data: Rc) -> Self { + pub(crate) fn new(id: impl Into, data: Arc) -> Self { Self { id: id.into(), data, diff --git a/tests/cli.rs b/tests/cli.rs index 3a62c69..7907c95 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -901,6 +901,47 @@ plot "b" { type = "line" query = query.revenue x = channel y = channel } assert!(message.contains("bad.dash:5"), "{message}"); assert!(message.contains("bad.dash:6"), "{message}"); + // A pie and a map check against the same database: the map's + // coordinates are found by name, and a text latitude is a mistake. + s.success(&[ + "query", + "--database", + "data.duckdb", + "--read-write", + "--sql", + "CREATE TABLE stations AS SELECT * FROM (VALUES + ('Utrecht', 'NL', 52.09, 5.11), ('Aachen', 'D', 50.77, 6.09) + ) t(name, country, geo_lat, geo_lng)", + ]); + std::fs::write( + s.0.join("geo.dash"), + r#" +query "stations" { sql = "SELECT * FROM stations" } +query "share" { + sql = "SELECT channel, sum(amount) AS total FROM usage GROUP BY channel" +} +plot "where" { type = "map" query = query.stations color = country } +plot "share" { type = "pie" query = query.share x = channel y = total } +"#, + ) + .unwrap(); + let out = s.object(&["check", "geo.dash", "--database", "data.duckdb"]); + assert_eq!(out["plots"][0]["type"], "map"); + assert_eq!(out["plots"][0]["color"], "country"); + assert_eq!(out["plots"][1]["type"], "pie"); + std::fs::write( + s.0.join("badgeo.dash"), + r#" +query "stations" { sql = "SELECT * FROM stations" } +plot "where" { type = "map" query = query.stations lat = name } +"#, + ) + .unwrap(); + let error = s.error(&["check", "badgeo.dash", "--database", "data.duckdb"], 2, "spec"); + let message = error["error"]["message"].as_str().unwrap(); + assert!(message.contains("lat = \"name\""), "{message}"); + assert!(message.contains("not numeric"), "{message}"); + // Everything wrong before a database opens is said without opening one: // syntax, unknown blocks, dangling references, multi-statement SQL. s.error(&["check"], 2, "argument"); diff --git a/tests/lsp.rs b/tests/lsp.rs index 58f8f50..4cb7e87 100644 --- a/tests/lsp.rs +++ b/tests/lsp.rs @@ -330,7 +330,7 @@ fn completion_inside_a_plot_offers_its_attributes() { .collect(); assert_eq!( labels, - ["type", "query", "x", "y", "series", "title"], + ["type", "query", "x", "y", "series", "title", "lat", "lng", "color"], "{response}" ); assert_eq!(items[0]["kind"], 10, "property: {response}");