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 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/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/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/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/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); 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}");