From a183748e2b10d51aaa5066dad7d15f8c2f89789a Mon Sep 17 00:00:00 2001 From: JetSquirrel Date: Mon, 28 Sep 2026 23:32:04 +0800 Subject: [PATCH 1/4] Upgrade to gpui-kit v0.7.0 Pin the toolkit, shell and component catalog to the v0.7.0 tag and open windows through gpui_kit::open_window instead of hand-wrapping a Root. The hidden export window gets the same Root as the main window, which fixes a panic when a .dash app raises a dialog during export. --- Cargo.lock | 131 ++++++++++++++++++++++-------------------- Cargo.toml | 11 ++-- src/app_export/run.rs | 17 ++++-- src/main.rs | 25 ++++---- 4 files changed, 99 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 30a0d33..5956fc9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2251,6 +2251,7 @@ dependencies = [ "sqlformat", "tracing", "tracing-subscriber", + "unicode-width", "ureq", ] @@ -3171,8 +3172,8 @@ dependencies = [ [[package]] name = "gpui-base" -version = "0.6.5" -source = "git+https://github.com/longbridge/gpui-kit?rev=13c716b687b47f96a677aaf46c8fa341fa7208da#13c716b687b47f96a677aaf46c8fa341fa7208da" +version = "0.7.0" +source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "aho-corasick", "anyhow", @@ -3186,9 +3187,11 @@ dependencies = [ "gpui-pre-sum-tree", "html5ever", "instant", + "itertools 0.13.0", "lsp-types", "markdown", "markup5ever_rcdom", + "num-traits", "objc2 0.6.4", "objc2-app-kit 0.3.2", "objc2-foundation 0.3.2", @@ -3208,8 +3211,8 @@ dependencies = [ [[package]] name = "gpui-component" -version = "0.6.5" -source = "git+https://github.com/longbridge/gpui-kit?rev=13c716b687b47f96a677aaf46c8fa341fa7208da#13c716b687b47f96a677aaf46c8fa341fa7208da" +version = "0.7.0" +source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "anyhow", "chrono", @@ -3255,8 +3258,8 @@ dependencies = [ [[package]] name = "gpui-component-macros" -version = "0.6.5" -source = "git+https://github.com/longbridge/gpui-kit?rev=13c716b687b47f96a677aaf46c8fa341fa7208da#13c716b687b47f96a677aaf46c8fa341fa7208da" +version = "0.7.0" +source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "proc-macro-crate", "proc-macro2", @@ -3266,8 +3269,8 @@ dependencies = [ [[package]] name = "gpui-component-shell" -version = "0.6.5" -source = "git+https://github.com/longbridge/gpui-kit?rev=13c716b687b47f96a677aaf46c8fa341fa7208da#13c716b687b47f96a677aaf46c8fa341fa7208da" +version = "0.7.0" +source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "gpui-component", "gpui-shell", @@ -3275,8 +3278,8 @@ dependencies = [ [[package]] name = "gpui-fps" -version = "0.6.5" -source = "git+https://github.com/longbridge/gpui-kit?rev=13c716b687b47f96a677aaf46c8fa341fa7208da#13c716b687b47f96a677aaf46c8fa341fa7208da" +version = "0.7.0" +source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "core-graphics 0.24.0", "gpui-pre", @@ -3292,8 +3295,8 @@ dependencies = [ [[package]] name = "gpui-kit" -version = "0.6.5" -source = "git+https://github.com/longbridge/gpui-kit?rev=13c716b687b47f96a677aaf46c8fa341fa7208da#13c716b687b47f96a677aaf46c8fa341fa7208da" +version = "0.7.0" +source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "gpui-base", "gpui-component", @@ -3305,8 +3308,8 @@ dependencies = [ [[package]] name = "gpui-kit-assets" -version = "0.6.5" -source = "git+https://github.com/longbridge/gpui-kit?rev=13c716b687b47f96a677aaf46c8fa341fa7208da#13c716b687b47f96a677aaf46c8fa341fa7208da" +version = "0.7.0" +source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "anyhow", "gpui-pre", @@ -3319,9 +3322,9 @@ dependencies = [ [[package]] name = "gpui-pre" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a0437c0b83e636a92bd1a39fa1d05fb632ae671289537497b35871ffbe231b84" +checksum = "0e87a42bb37c7cb4e76dd1ac0ce88851e46e976e0373a47ab3e0757abffee54d" dependencies = [ "accesskit", "anyhow", @@ -3390,12 +3393,12 @@ dependencies = [ [[package]] name = "gpui-pre-apple" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3af534f0746eb9e014114bcc0ee00dc250c4b8d75df2e437180c403aa65649c1" +checksum = "00053551517815ab169fda28fff78dbebf2712722593c7671c511785cb3f79d5" dependencies = [ "anyhow", - "block", + "block2 0.6.2", "cbindgen", "cocoa 0.26.0", "core-foundation 0.10.1", @@ -3414,9 +3417,9 @@ dependencies = [ [[package]] name = "gpui-pre-collections" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "15e7a65204039187d1f4b3d4c2e0233eeac54d4acb0b13f9dc1a3b5adad26c1f" +checksum = "f79760925c31ed06924d28d68981b5b22b2ce25665f58f0a33dfea11170e6e28" dependencies = [ "gpui-pre-util", "indexmap", @@ -3425,9 +3428,9 @@ dependencies = [ [[package]] name = "gpui-pre-derive-refineable" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "79dfb4a05f3cd9893d7768dbf5dba1bbbe99cab7394e89b360ccb3b2bcb6b34e" +checksum = "d1aea3f941581a6a9dd545ecd351ef2d336fe6e2a1a4555cb6414944a4984260" dependencies = [ "proc-macro2", "quote", @@ -3436,9 +3439,9 @@ dependencies = [ [[package]] name = "gpui-pre-http-client" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cfd5a7542c77887a5aff997d78c95884119c8463409569b63fcf060956da1682" +checksum = "ddce27e989e877a3a1a243f97d7ea4f1ade2532abec418d5a0be9cca802702c7" dependencies = [ "anyhow", "async-compression", @@ -3457,9 +3460,9 @@ dependencies = [ [[package]] name = "gpui-pre-linux" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aab655cda1d6dcd549ac869e8678babc84cf573ab6794a1fe1a39c6c70c60b48" +checksum = "0aac7022e347409454777701082201742710052813964a1e25160f7e22c965ad" dependencies = [ "accesskit", "accesskit_unix", @@ -3504,9 +3507,9 @@ dependencies = [ [[package]] name = "gpui-pre-macos" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "951d17e41a72067ad2d35c60e08a62e8ad7fa9400b41c802e26cfb459f6c05da" +checksum = "5a43af845b260b09393e923c4f1e1a67a10fcfc847b4815192bbfd02ed9fe725" dependencies = [ "accesskit", "accesskit_macos", @@ -3551,9 +3554,9 @@ dependencies = [ [[package]] name = "gpui-pre-macros" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6324092b40ea8ec7c327a4a28427e366a489b4d5d23b5d1bd0243fc2f4ca7a9e" +checksum = "e1627c5dd3c3351e39e0621d3f3e31dbbe7c7658df90b1d2ec12bfe95d54cd37" dependencies = [ "heck 0.5.0", "proc-macro-crate", @@ -3564,9 +3567,9 @@ dependencies = [ [[package]] name = "gpui-pre-perf" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b94bff26ccd8105f6206144f8dab16a2dc02e7a68f39dada02cdf492f0afd940" +checksum = "01954497dd02ba96ab4c252a8668643b0cc3085e7a75dca62e7ba0e3dcd83b02" dependencies = [ "gpui-pre-collections", "serde", @@ -3575,10 +3578,11 @@ dependencies = [ [[package]] name = "gpui-pre-platform" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8423d1e0a693d2f9f16c0326bb01b5e81ec0531854d596a18476934ac6880478" +checksum = "0112105c2ac6757fbf527c9c92a753be9bcbce9f4cf26959a11eaa1db6565aa5" dependencies = [ + "anyhow", "console_error_panic_hook", "gpui-pre", "gpui-pre-linux", @@ -3589,9 +3593,9 @@ dependencies = [ [[package]] name = "gpui-pre-refineable" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "93b4b0a8d296a3ee5beb1136bc48f8167a6028eb1e243b56851dcd8c9fc6d7a1" +checksum = "ffd14639e5dfab906bbaeabfe0f94289120dcf2e6ed37217deb13964867e89a5" dependencies = [ "gpui-pre-derive-refineable", ] @@ -3649,9 +3653,9 @@ dependencies = [ [[package]] name = "gpui-pre-scheduler" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff7b10b3d2fc3fda24847a04436eca9531612c8414874a37ddc005f20e521e12" +checksum = "8049963b716df01b67b58f86000597f222d5b31b15b35069b23a3b07f3c68352" dependencies = [ "async-task", "backtrace", @@ -3666,9 +3670,9 @@ dependencies = [ [[package]] name = "gpui-pre-shared-string" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe17f81bd98a0253140ea79f7f76442d7499bd8d17db302861af8e46c11d38df" +checksum = "8ef6314bdf5ab162c1913712ec4a91b013ce2a9cfb0b557ecdfdd5793c8b61d3" dependencies = [ "schemars", "serde", @@ -3677,9 +3681,9 @@ dependencies = [ [[package]] name = "gpui-pre-sum-tree" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a6d35e998a51d24248dad731de1f09d386cbbf46a04edde3b67f469b2ce3f16" +checksum = "5cca7e5fad9d53b2d265860fefdffc5f171966cb99c3fd99ecac15c033f19209" dependencies = [ "gpui-pre-ztracing", "heapless", @@ -3690,9 +3694,9 @@ dependencies = [ [[package]] name = "gpui-pre-util" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a026a30c578930d20974fd8c3d77de36223ac7ba193bd4259e7238929bc8806" +checksum = "d82e06359ae76f8bb07ed713c54adb80cea6a9a2d3dc715f41aec30cd2aadb4d" dependencies = [ "anyhow", "log", @@ -3701,9 +3705,9 @@ dependencies = [ [[package]] name = "gpui-pre-util-macros" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5cbf79627a6afc626d4e887dad9d2d92bb069413e8cc692ba3e90bff219eb7a9" +checksum = "b8a0da4143bb0e7eb926eed7edd3a61c9d17344207543dcc25fb398f0cb96b2f" dependencies = [ "gpui-pre-perf", "quote", @@ -3712,9 +3716,9 @@ dependencies = [ [[package]] name = "gpui-pre-web" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34be433b9fcb370b1fe9a1c7fe911f0eb6058791ef8167dcf6c5a5deb624975c" +checksum = "ab3870cc909471bb830c3d79496770c754401aad6cfa122147ccb13e8908ad08" dependencies = [ "anyhow", "console_error_panic_hook", @@ -3741,9 +3745,9 @@ dependencies = [ [[package]] name = "gpui-pre-wgpu" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05515b32799b1767d69300921c1b562cd53104965211a0e8905acdc7aa18e693" +checksum = "f0b02657b56b09140ce542f5e4434fb96d9fdaba0fd0035ba7dba13c171ddb9b" dependencies = [ "anyhow", "bytemuck", @@ -3768,9 +3772,9 @@ dependencies = [ [[package]] name = "gpui-pre-windows" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "27662e65bfcf4445d2cccfad91f1570ec07f4b1b5de98400a9f207fd19c464cf" +checksum = "f05592a6f9e3e6bb7a9f2a0d4779271cf747a948db1c07b02448de777ffff8f3" dependencies = [ "accesskit", "accesskit_windows", @@ -3797,9 +3801,9 @@ dependencies = [ [[package]] name = "gpui-pre-zlog" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3a5af0b734a391a84872eb770b3ee9830091ec2487b79d06371fe14eefb2b5d" +checksum = "a7506302091e66d7fb5e04a5b3480e90cff0bbe3157458d7df91a41c59238c48" dependencies = [ "anyhow", "chrono", @@ -3809,9 +3813,9 @@ dependencies = [ [[package]] name = "gpui-pre-ztracing" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a24cb19f78c5bc3aab8ddf1e8e0bd6b402fbcb9c7f112f64ddbedef8f8768a59" +checksum = "4ed1bc6d53b3ca93785a8573d1066d555b67bc475b344fb28871f9811eb6f218" dependencies = [ "gpui-pre-zlog", "gpui-pre-ztracing-macro", @@ -3821,14 +3825,14 @@ dependencies = [ [[package]] name = "gpui-pre-ztracing-macro" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1bb25850ad31b88bbdef454afb93ec4cdb81f5e200ba5b3f1a0368111780d2c" +checksum = "829258540ea51bb9ea4a71686beffe5c99586901ba9f39944d06e4a180ab63fe" [[package]] name = "gpui-shell" -version = "0.6.5" -source = "git+https://github.com/longbridge/gpui-kit?rev=13c716b687b47f96a677aaf46c8fa341fa7208da#13c716b687b47f96a677aaf46c8fa341fa7208da" +version = "0.7.0" +source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "anyhow", "async-channel", @@ -3840,6 +3844,7 @@ dependencies = [ "gpui-pre", "gpui-pre-platform", "gpui-pre-reqwest", + "image", "instant", "libc", "quickjs-jit", @@ -3851,8 +3856,10 @@ dependencies = [ "serde_json", "sha2 0.10.9", "smallvec", + "smol", "tracing", "tungstenite", + "usvg 0.46.0", "wait-timeout", "windows 0.58.0", ] @@ -7430,7 +7437,7 @@ dependencies = [ [[package]] name = "rquickjs" version = "0.12.9" -source = "git+https://github.com/longbridge/gpui-kit?rev=13c716b687b47f96a677aaf46c8fa341fa7208da#13c716b687b47f96a677aaf46c8fa341fa7208da" +source = "git+https://github.com/longbridge/gpui-kit?tag=v0.7.0#0c830f4d257e69fdd17200650533ab4ca9a40cc0" dependencies = [ "quickjs-jit", ] diff --git a/Cargo.toml b/Cargo.toml index 25c6f15..154307e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,14 +12,14 @@ categories = ["database", "gui"] [dependencies] # One dependency graph: the UI toolkit, the scriptable-shell runtime and the -# component catalog all come from the same pinned revision. Mixing the +# component catalog all come from the same release tag. Mixing the # crates.io release with the Git one would put two incompatible copies of # `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", rev = "13c716b687b47f96a677aaf46c8fa341fa7208da", features = ["tree-sitter-sql"] } -gpui-shell = { git = "https://github.com/longbridge/gpui-kit", rev = "13c716b687b47f96a677aaf46c8fa341fa7208da" } -gpui-component-shell = { git = "https://github.com/longbridge/gpui-kit", rev = "13c716b687b47f96a677aaf46c8fa341fa7208da" } +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" } duckdb = { version = "1", features = ["bundled", "json", "parquet"] } calamine = { version = "0.36", features = ["dates"] } smol = "2" @@ -37,6 +37,7 @@ ureq = { version = "3", default-features = false, features = ["rustls"] } hmac = "0.12" sha2 = "0.10" quick-xml = { version = "0.36", features = ["serialize"] } +unicode-width = "0.2" [dev-dependencies] rust_xlsxwriter = "0.99" @@ -44,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", rev = "13c716b687b47f96a677aaf46c8fa341fa7208da" } +rquickjs = { git = "https://github.com/longbridge/gpui-kit", tag = "v0.7.0" } # 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/src/app_export/run.rs b/src/app_export/run.rs index 4c0bb1e..7049b22 100644 --- a/src/app_export/run.rs +++ b/src/app_export/run.rs @@ -56,10 +56,13 @@ pub const DEFAULT_TIMEOUT: Duration = Duration::from_secs(15); /// that waits on it can see whether the mount worked. type MountSlot = Rc, String>>>>; -/// The window's root: the app, when it mounted, and nothing when it did not. +/// The window's content: the app, when it mounted, and nothing when it did not. /// -/// A window needs a root view, and the root has to be one type whether the -/// app loaded or not — so this holds either. +/// `gpui_kit::open_window` wraps this in the `Root` every workspace window has, +/// so a script component that raises a dialog or a notification finds the host +/// it would find on screen instead of panicking on a root it cannot name. The +/// content has to be one type whether the app loaded or not, so this holds +/// either. struct AppHost(Option>); impl Render for AppHost { @@ -144,7 +147,11 @@ pub fn capture(job: Job, finish: impl FnOnce(Outcome) -> std::convert::Infallibl let slot = mounted.clone(); let app = job.app.clone(); let options = hidden_window(cx); - let window = match cx.open_window(options, move |window, cx| { + // The same entry point the main window uses. `runtime::create` + // above already ran the component initializer the helper asks for, + // so the Root it wraps around the host mounts the overlay layers a + // .dash app can ask for, exactly as on screen. + let window = match gpui_kit::open_window(options, cx, move |window, cx| { // The app's folder answers `appDir()` for this call and for // every host call `init` makes inside it. let view = host::with_panel_directory(&app, || { @@ -161,7 +168,7 @@ pub fn capture(job: Job, finish: impl FnOnce(Outcome) -> std::convert::Infallibl } } }) { - Ok(window) => window, + Ok((window, _)) => window, Err(error) => stop!(Outcome::Failed(format!("{error:#}"))), }; // Opening the window drew one frame, so an app that renders diff --git a/src/main.rs b/src/main.rs index 3a7bbc5..8307ba6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,7 +20,7 @@ mod spec; mod state; mod ui; -use gpui_kit::component::{Root, Theme, ThemeMode, TitleBar}; +use gpui_kit::component::{Theme, ThemeMode, TitleBar}; use gpui_kit::*; use crate::app::DuckLocalApp; @@ -68,18 +68,17 @@ fn main() { ui::scale::apply(cx); let window_bounds = Bounds::centered(None, size(px(1440.), px(900.)), cx); - cx.spawn(async move |cx| { - let options = WindowOptions { - window_bounds: Some(WindowBounds::Windowed(window_bounds)), - window_min_size: Some(size(px(960.), px(600.))), - ..TitleBar::window_options() - }; - cx.open_window(options, |window, cx| { - let view = cx.new(|cx| DuckLocalApp::new(paths, window, cx)); - cx.new(|cx| Root::new(view, window, cx)) - }) - .expect("Failed to open window"); + let options = WindowOptions { + window_bounds: Some(WindowBounds::Windowed(window_bounds)), + window_min_size: Some(size(px(960.), px(600.))), + ..TitleBar::window_options() + }; + // The helper wraps the view in the Root that hosts overlays and + // window chrome; the run closure already holds `&mut App`, so no + // spawn is needed to reach one. + gpui_kit::open_window(options, cx, move |window, cx| { + cx.new(|cx| DuckLocalApp::new(paths, window, cx)) }) - .detach(); + .expect("Failed to open window"); }); } From 7a57fa742fef3f2fbc2abaefcb27a1bc1d49a276 Mon Sep 17 00:00:00 2001 From: JetSquirrel Date: Mon, 28 Sep 2026 23:32:11 +0800 Subject: [PATCH 2/4] Underline the failing span when a query errors Parse DuckDB's LINE n caret position into a UTF-8 byte range and mark it in the editor with a wavy underline and a faint fill, both in the theme's danger color, through v0.7.0's decoration collections. The marks follow edits, stay out of undo history, and clear on the next run. A failed run is captured per tab, so a late answer lands in the tab that asked. --- src/query.rs | 184 ++++++++++++++++++++++++++++++++++++++++++++ src/ui/workspace.rs | 109 ++++++++++++++++++++++---- 2 files changed, 279 insertions(+), 14 deletions(-) diff --git a/src/query.rs b/src/query.rs index d780558..413d8a7 100644 --- a/src/query.rs +++ b/src/query.rs @@ -2,6 +2,7 @@ //! //! Blocking functions; call via `smol::unblock` from UI code. +use std::ops::Range; use std::time::Instant; use anyhow::Result; @@ -517,6 +518,96 @@ pub fn run_of(conn: &Connection, sql: &str) -> Result { })) } +/// The UTF-8 byte range in `sql` that an error `message` points at, if it +/// points at all. +/// +/// Parser and binder errors render their position as a caret under the +/// offending line: +/// +/// ```text +/// Catalog Error: Table with name t does not exist! +/// +/// LINE 2: from t +/// ^ +/// ``` +/// +/// The caret column counts *characters*, and the caret line is indented by +/// the whole `LINE n: ` label, so the label width comes off before the column +/// means anything. One caret marks where the token starts, so the range +/// extends over the token; a run of carets already spans it. Errors without a +/// position (runtime failures, a batch whose bind error carries none) return +/// `None`, and a rendered line that does not match the SQL — a DuckDB that +/// truncates long lines would produce one — yields `None` rather than a +/// squiggle in the wrong place. +pub fn error_byte_range(sql: &str, message: &str) -> Option> { + let mut rendered = message.lines(); + while let Some(header) = rendered.next() { + let Some(rest) = header.strip_prefix("LINE ") else { + continue; + }; + let Some((digits, shown)) = rest.split_once(": ") else { + continue; + }; + let Ok(number) = digits.parse::() else { + continue; + }; + let caret = rendered.next().unwrap_or(""); + let padding = caret.len() - caret.trim_start_matches(' ').len(); + let label = "LINE ".len() + digits.len() + ": ".len(); + let column = padding.checked_sub(label)?; + let carets = caret + .trim_start_matches(' ') + .chars() + .take_while(|&c| c == '^') + .count(); + + // The line the number refers to, in the SQL that was run. + let mut line_start = 0; + for _ in 1..number { + line_start = sql[line_start..].find('\n').map(|ix| line_start + ix + 1)?; + } + let line_end = sql[line_start..] + .find('\n') + .map(|ix| line_start + ix) + .unwrap_or(sql.len()); + let line = &sql[line_start..line_end]; + // Trust the position only while the rendered line is the SQL's own; + // otherwise the column maps to text the user never wrote. + if shown != line { + continue; + } + + let byte_of_char = |index: usize| { + line.char_indices() + .nth(index) + .map(|(ix, _)| ix) + .unwrap_or(line.len()) + }; + let start = line_start + byte_of_char(column); + let end = if carets > 1 { + line_start + byte_of_char(column + carets) + } else { + // One caret marks where the token starts; underline the token. + let mut end = start; + for ch in sql[start..line_end].chars() { + if ch.is_whitespace() { + break; + } + end += ch.len_utf8(); + } + end + }; + if start < end { + return Some(start..end); + } + // A caret at or past the line's end marks nothing on its own; fall + // back to the last character, which is where the caret was read. + let (ix, ch) = line.char_indices().next_back()?; + return Some(line_start + ix..line_start + ix + ch.len_utf8()); + } + None +} + /// `EXPLAIN ` rendered as plain text lines. pub fn explain_of(conn: &Connection, sql: &str) -> Result<(Vec, u128)> { let started = Instant::now(); @@ -850,6 +941,79 @@ mod tests { Connection::open_in_memory().unwrap() } + /// The message shapes here are real DuckDB renderings; the assertions are + /// byte ranges into the SQL the editor holds. + #[test] + fn error_positions_map_to_sql_bytes() { + // A binder error names the column; one caret underlines the token. + assert_eq!( + error_byte_range( + "select frum t", + "Binder Error: Referenced column \"frum\" was not found\n\nLINE 1: select frum t\n ^", + ), + Some(7..11) + ); + + // A later line, and a label wider than one digit. + assert_eq!( + error_byte_range( + "-- 1\n-- 2\n-- 3\n-- 4\n-- 5\n-- 6\n-- 7\n-- 8\n-- 9\nselect nope", + "Binder Error: Referenced column \"nope\" was not found\n\nLINE 10: select nope\n ^", + ), + Some(52..56) + ); + + // The caret counts characters, not bytes: `é` is two bytes, so the + // byte range sits one past the character column. + assert_eq!( + error_byte_range( + "select 'aé' as x, nope", + "Binder Error: Referenced column \"nope\" was not found\n\nLINE 1: select 'aé' as x, nope\n ^", + ), + Some(19..23) + ); + + // A run of carets already spans the token, including its last byte. + assert_eq!( + error_byte_range( + "select 1\nfrom missing", + "Catalog Error: Table with name missing does not exist!\n\nLINE 2: from missing\n ^^^^^^^", + ), + Some(14..21) + ); + + // A caret at the end of the line still marks the last character. + assert_eq!( + error_byte_range( + "select 'aé' +", + "Binder Error: No function matches ...\n\nLINE 1: select 'aé' +\n ^", + ), + Some(13..14) + ); + } + + #[test] + fn errors_without_a_position_mark_nothing() { + assert_eq!( + error_byte_range("select *", "Parser Error: syntax error at end of input"), + None + ); + // A rendered line that is not the SQL's own — as a truncation of a + // long line would read — is refused rather than misplaced. + assert_eq!( + error_byte_range( + "select frum t", + "Parser Error: ...\n\nLINE 1: select …\n ^", + ), + None + ); + // The line number must exist in the SQL that was run. + assert_eq!( + error_byte_range("select 1", "Parser Error: ...\n\nLINE 9: select 1\n ^"), + None + ); + } + #[test] fn select_returns_typed_columns() { let conn = mem(); @@ -1248,4 +1412,24 @@ mod tests { "_No columns were returned._\n" ); } + + /// The crafted messages above pin the parsing; this pins the parsing + /// against the messages the bundled DuckDB actually renders. + #[test] + fn real_duckdb_errors_map_to_the_span_they_render() { + let conn = mem(); + let sql = "select 1\nfrom missing"; + let error = run_of(&conn, sql).unwrap_err().to_string(); + assert_eq!(error_byte_range(sql, &error), Some(14..21), "{error}"); + + // Multibyte text before the caret: the column counts characters. + let sql = "select 'aé' as x, nope"; + let error = run_of(&conn, sql).unwrap_err().to_string(); + assert_eq!(error_byte_range(sql, &error), Some(19..23), "{error}"); + + // A runtime error renders no position, so nothing is marked. + let sql = "select error('boom')"; + let error = run_of(&conn, sql).unwrap_err().to_string(); + assert_eq!(error_byte_range(sql, &error), None, "{error}"); + } } diff --git a/src/ui/workspace.rs b/src/ui/workspace.rs index 6b4a027..bb39901 100644 --- a/src/ui/workspace.rs +++ b/src/ui/workspace.rs @@ -15,7 +15,10 @@ use std::rc::Rc; use gpui_kit::component::button::{Button, ButtonVariants}; use gpui_kit::component::dialog::DialogFooter; -use gpui_kit::component::input::{Editor, EditorState, Input, InputEvent, InputState, TabSize}; +use gpui_kit::component::input::{ + Editor, EditorState, Input, InputEvent, InputState, RangeDecoration, RangeDecorationCollection, + RangeDecorationStyle, TabSize, TextDecoration, TextDecorationCollection, +}; use gpui_kit::component::kbd::Kbd; use gpui_kit::component::menu::{DropdownMenu, PopupMenuItem}; use gpui_kit::component::notification::Notification; @@ -54,6 +57,13 @@ pub struct QueryTab { pub id: u64, pub title: SharedString, pub editor: Entity, + /// Where the last failed run's error landed, underlined in the editor. + /// The collections live on the tab rather than inside the run so a new + /// run clears them before it starts, without the editor in scope; the + /// ranges themselves follow edits and never enter undo history. + pub error_squiggles: TextDecorationCollection, + /// The quiet fill behind the same span. + pub error_fill: RangeDecorationCollection, /// This tab's own results. One panel shared by every tab showed tab A's /// rows under tab B's editor — and exported them with A's SQL. pub results: Entity, @@ -249,6 +259,16 @@ impl WorkspaceTab { } } +/// One run's delivery target: the SQL as run, where its outcome is shown, +/// and the editor decorations an error is marked with. Captured before the +/// run starts so a late answer still lands in the tab that asked. +struct ActiveQuery { + sql: String, + squiggles: TextDecorationCollection, + fill: RangeDecorationCollection, + results: Entity, +} + pub struct Workspace { state: Entity, tabs: Vec, @@ -332,9 +352,13 @@ impl Workspace { }) }); let provider = completion::SqlCompletionProvider::new(self.state.clone()); - editor.update(cx, |state, cx| { + let (error_squiggles, error_fill) = editor.update(cx, |state, cx| { state.lsp_mut().completion_provider = Some(provider); cx.notify(); + ( + state.create_decorations_collection(Vec::new(), cx), + state.create_range_decorations_collection(Vec::new(), cx), + ) }); // ⌘↵ reaches the editor as `secondary-enter`, which the Input key // context (deeper than the workspace's) binds to "insert newline". @@ -358,6 +382,8 @@ impl Workspace { id, title: trf("workspace.tab.default_title", &[&id.to_string()]).into(), editor, + error_squiggles, + error_fill, results: cx.new(|cx| ResultsPanel::new(window, cx)), } } @@ -811,16 +837,23 @@ impl Workspace { self.save_dashboard(tab_id, window, cx); } - /// The active query's SQL, with the results panel its outcome belongs to: - /// captured when the run starts, so a result that lands after the user - /// switched tabs still goes to the tab that asked. - fn active_sql(&self, cx: &App) -> Option<(String, Entity)> { + /// The active query's SQL, with everything a run delivers into: the + /// results panel the outcome lands in, and the decoration collections an + /// error is marked with. All of it is captured when the run starts, so a + /// result that lands after the user switched tabs still goes to the tab + /// that asked. + fn active_sql(&self, cx: &App) -> Option { let tab = self .tabs .get(self.active) .and_then(WorkspaceTab::as_query)?; let sql = tab.editor.read(cx).value().to_string(); - (!sql.trim().is_empty()).then(|| (sql, tab.results.clone())) + (!sql.trim().is_empty()).then(|| ActiveQuery { + sql, + squiggles: tab.error_squiggles.clone(), + fill: tab.error_fill.clone(), + results: tab.results.clone(), + }) } fn open_rename_dialog(&mut self, _: &ClickEvent, window: &mut Window, cx: &mut Context) { @@ -901,16 +934,23 @@ impl Workspace { if self.running || self.explaining { return; } - let Some((sql, results)) = self.active_sql(cx) else { + let Some(query) = self.active_sql(cx) else { return; }; // ⌘↵ on the first-run screen means "I want to write SQL": show the // editor the results belong to instead of running behind it. self.editor_shown = true; self.running = true; - results.update(cx, |results, cx| results.set_running(cx)); + // The previous run's mark names a range in SQL that is about to be + // judged again; stale red is worse than none. + query.squiggles.clear(cx); + query.fill.clear(cx); + query + .results + .update(cx, |results, cx| results.set_running(cx)); cx.notify(); + let sql = query.sql.clone(); let state = self.state.clone(); cx.spawn_in(window, async move |this, cx| { let run_sql = sql.clone(); @@ -964,7 +1004,10 @@ impl Workspace { }), Err(_) => None, }; - results.update(cx, |results, cx| { + if let Err(error) = &outcome { + Self::mark_sql_error(&query, error, cx); + } + query.results.update(cx, |results, cx| { results.set_outcome(outcome, sql.clone(), window, cx); }); state.update(cx, |s, cx| { @@ -1004,26 +1047,64 @@ impl Workspace { }); } + /// Underline the span a failed run's error named, if it named one: a wavy + /// underline on the token and a quiet fill behind it, both in the theme's + /// danger color. The collections follow edits and sit outside undo + /// history, so nothing here is replayed by ⌘Z. + fn mark_sql_error(query: &ActiveQuery, error: &anyhow::Error, cx: &mut App) { + let Some(range) = crate::query::error_byte_range(&query.sql, &error.to_string()) else { + return; + }; + let danger = cx.theme().danger; + query.squiggles.set( + vec![TextDecoration::new( + range.clone(), + HighlightStyle { + underline: Some(UnderlineStyle { + color: Some(danger), + thickness: px(1.), + wavy: true, + }), + ..Default::default() + }, + )], + cx, + ); + query.fill.set( + vec![RangeDecoration::new(range) + .with_style(RangeDecorationStyle::Fill) + .with_color(danger.opacity(0.1))], + cx, + ); + } + fn explain_active(&mut self, _: &ClickEvent, window: &mut Window, cx: &mut Context) { if self.explaining || self.running { return; } - let Some((sql, results)) = self.active_sql(cx) else { + let Some(query) = self.active_sql(cx) else { return; }; self.explaining = true; - results.update(cx, |results, cx| results.set_running(cx)); + query.squiggles.clear(cx); + query.fill.clear(cx); + query + .results + .update(cx, |results, cx| results.set_running(cx)); cx.notify(); cx.spawn_in(window, async move |this, cx| { - let explain_sql = sql.clone(); + let explain_sql = query.sql.clone(); let result = smol::unblock(move || { crate::db::with_connection(|conn| crate::query::explain_of(conn, &explain_sql)) }) .await; this.update_in(cx, move |this, window, cx| { this.explaining = false; - results.update(cx, |results, cx| { + if let Err(error) = &result { + Self::mark_sql_error(&query, error, cx); + } + query.results.update(cx, |results, cx| { results.set_explain(result, window, cx); }); cx.notify(); From 66ab43eb839cfbd517c845d2ac90fc24f91a387e Mon Sep 17 00:00:00 2001 From: JetSquirrel Date: Mon, 28 Sep 2026 23:32:20 +0800 Subject: [PATCH 3/4] Draw dashboard charts honestly, and polish the tables around them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A pivoted bar spec now draws one grouped chart instead of one small chart per series, scatter draws unconnected points, and a pivoted line draws lines without the borrowed area fill — all hand-built on gpui_kit::base::plot, which v0.7.0 publishes without the styled layer. - Chart tooltips show the formatted value (4.4M) instead of the raw f64, and x labels spread by a width-aware count so date labels no longer collide at the edges. - Result and dashboard tables fit column widths to sampled content, CJK-aware, migrated from docs-onboarding (adds unicode-width). - The title bar and export row compose as Toolbar/ToolbarGroup, gaining arrow-key focus traversal. --- src/spec/mod.rs | 2 + src/spec/plot.rs | 633 ++++++++++++++++++++++++++++++++++++++++++++ src/spec/view.rs | 113 +++----- src/ui/results.rs | 113 ++++++-- src/ui/title_bar.rs | 44 ++- 5 files changed, 780 insertions(+), 125 deletions(-) create mode 100644 src/spec/plot.rs diff --git a/src/spec/mod.rs b/src/spec/mod.rs index 5872587..9791b0e 100644 --- a/src/spec/mod.rs +++ b/src/spec/mod.rs @@ -11,6 +11,7 @@ //! src/spec/model.rs what the tree means, checked without a database //! src/spec/mod.rs `ducklocal check`, the command half //! src/spec/prepare.rs a plot's data, derived from its query's result once +//! src/spec/plot.rs the plots the catalog charts cannot draw honestly //! src/spec/highlight.rs colours for the source editor, SQL heredocs included //! src/spec/view.rs the view half: a `.dash` file as a workspace tab //! src/spec/tabs.rs which specs are open, remembered between launches @@ -30,6 +31,7 @@ pub mod complete; pub mod highlight; pub mod lsp; pub mod model; +pub mod plot; pub mod prepare; pub mod syntax; pub mod tabs; diff --git a/src/spec/plot.rs b/src/spec/plot.rs new file mode 100644 index 0000000..738e0d3 --- /dev/null +++ b/src/spec/plot.rs @@ -0,0 +1,633 @@ +//! The dashboard plots the catalog charts cannot draw. +//! +//! The catalog's `BarChart` and `LineChart` take a single series, and its one +//! multi-series chart fills areas — so a pivoted `bar` drawn with them comes +//! out as a stack of small charts, a pivoted `line` as a faintly filled area +//! and a `scatter` as dots connected by lines. The two plots here draw those +//! on the same `plot` primitives the catalog composes: [`GroupedBars`] lays a +//! band's series side by side within the band, and [`SeriesPlot`] projects +//! every series onto shared point and value scales, connecting a series' +//! points (a `line`) or not (a `scatter`). +//! +//! Composition follows the catalog charts — same axis gutter, grid, label +//! stride, palette, crosshair and tooltip — so a dashboard reads as one family +//! whichever kind a plot is. Like them, a plot here is a value rebuilt on +//! every frame: hover state and the line path caches live in the window's +//! element state, keyed on the plot's id. And the data is `prepare`'s, already +//! capped at its bar/point/series limits, so a frame's work stays bounded the +//! way a catalog chart's is. + +use std::sync::Arc; + +use gpui_kit::component::ActiveTheme; +use gpui_kit::component::plot::label::{Text, TEXT_GAP, TEXT_SIZE}; +use gpui_kit::component::plot::scale::{Scale, ScaleBand, ScaleLinear, ScalePoint}; +use gpui_kit::component::plot::shape::{Bar, Line}; +use gpui_kit::component::plot::tooltip::{CrossLine, Dot, Tooltip, TooltipState}; +use gpui_kit::component::plot::{ + AxisText, Grid, PathCaches, Plot, PlotAxis, PlotElement, PlotLabel, axis_gutter, +}; +use gpui_kit::*; + +use crate::spec::prepare::{PlotPoint, PreparedPlot}; +use crate::ui::chart::format_value; + +/// The headroom kept above the tallest bar or point, as the catalog charts +/// keep above theirs. +const TOP_GAP: f32 = 10.; +/// The widest one bar in a group: the catalog caps a single-series bar at +/// 30px, and a grouped bar is no different. +const MAX_BAR_WIDTH: f32 = 30.; +/// The gap between two bars of one group: enough to tell the series apart, +/// not enough to split the group. +const GROUP_GAP: f32 = 2.; +/// The dot a scatter paints per point, and the one the tooltip marks a hovered +/// point with — sized as the catalog's dotted line and hover dots are. +const DOT_SIZE: f32 = 8.; +/// The ring behind a hovered dot at full hover, as the catalog draws it. +const HOVER_HALO: f32 = 20.; +/// The value-axis ticks the grid is drawn at, the catalog's default. +const TICK_COUNT: usize = 5; + +/// The gutter a plot reserves under itself for its x-axis labels, as the +/// catalog computes it for the labels' font size. +fn axis_gap() -> f32 { + axis_gutter(px(TEXT_SIZE)) +} + +/// The dashboard's chart palette: series cycle `chart_1..chart_5`, as at the +/// catalog call sites. +fn palette(cx: &App) -> [Hsla; 5] { + let theme = cx.theme(); + [ + theme.chart_1, + theme.chart_2, + theme.chart_3, + theme.chart_4, + theme.chart_5, + ] +} + +/// Within a band of `band_width`, the width of one of `series` side-by-side +/// bars and the offset of the `s`-th from the band's left edge. +fn group_slot(band_width: f32, series: usize, s: usize) -> (f32, f32) { + let n = series.max(1) as f32; + let width = ((band_width - GROUP_GAP * (n - 1.)) / n).max(0.); + (width, s as f32 * (width + GROUP_GAP)) +} + +/// `count` evenly spaced value-axis positions, from the plot's top edge to +/// the baseline, both included; the last is the baseline the axis itself draws. +fn value_ticks(top: f32, baseline: f32, count: usize) -> Vec { + let count = count.max(2); + let steps = (count - 1) as f32; + (0..count) + .map(|i| top + (baseline - top) * i as f32 / steps) + .collect() +} + +/// How many x labels fit without neighbours touching: a label needs its text +/// width — roughly 8px per character at the axis font — plus padding, and the +/// plot is at least 600px wide (the minimum window minus sidebar, padding and +/// the axis gutter). Feeding a count rather than a stride lets the labeling +/// below keep the first and last value, where a stride drops the first. +pub(crate) fn x_label_count(points: &[Arc]) -> usize { + const MIN_PLOT_WIDTH: f32 = 600.; + const MAX_LABELS: usize = 12; + let widest = points + .iter() + .map(|p| p.band.chars().count()) + .max() + .unwrap_or(0) as f32; + let slot = widest * 8. + 24.; + ((MIN_PLOT_WIDTH / slot) as usize).clamp(2, MAX_LABELS) +} + +/// Which of `len` items carry an x-axis label: `count` of them spread evenly +/// from the first to the last, as the catalog charts place a tick count. +fn labeled(len: usize, count: usize) -> Vec { + let mut labeled = vec![false; len]; + match count.min(len) { + 0 => {} + 1 => labeled[0] = true, + n if n >= len => labeled.iter_mut().for_each(|l| *l = true), + n => { + for k in 0..n { + let ix = (k as f32 * (len - 1) as f32 / (n - 1) as f32).round() as usize; + labeled[ix] = true; + } + } + } + labeled +} + +/// The alignment of the `i`-th of `len` x labels: the first hugs the left +/// edge, the last the right, the rest center on their tick, as the catalog's +/// point charts place them. +fn point_label_align(i: usize, len: usize) -> TextAlign { + match i { + 0 if len == 1 => TextAlign::Center, + 0 => TextAlign::Left, + i if i == len - 1 => TextAlign::Right, + _ => TextAlign::Center, + } +} + +/// A multi-series `bar`: one chart, one band per x value, the series side by +/// side within the band — the layout the catalog's single-series `BarChart` +/// cannot express. Bars are plain quads, cheap enough to paint uncached; the +/// hover band and the tooltip come from the plot's id. +pub(crate) struct GroupedBars { + id: ElementId, + points: Vec>, + series: Vec, + label_count: usize, +} + +impl GroupedBars { + pub(crate) fn new(id: impl Into, plot: &PreparedPlot) -> Self { + Self { + id: id.into(), + points: plot.points.clone(), + series: plot + .series_names + .iter() + .map(SharedString::from) + .collect(), + label_count: x_label_count(&plot.points), + } + } + + /// The band and value scales for `bounds`, shared by `paint` and the + /// tooltip so the bars and the hover band stay aligned. The value scale + /// spans the data and zero, so a bar always grows from the zero line. + fn scales(&self, bounds: Bounds) -> (ScaleBand, ScaleLinear) { + let baseline = bounds.size.height.as_f32() - axis_gap(); + // The paddings are the catalog `BarChart`'s, so a band sits where a + // single-series chart would put it; the cap keeps each bar of the + // group at most MAX_BAR_WIDTH, however wide the plot. + let band = ScaleBand::new( + self.points.iter().map(|d| d.band.clone()), + [0., bounds.size.width.as_f32()], + ) + .padding_inner(0.4) + .padding_outer(0.2) + .max_band_width(MAX_BAR_WIDTH * self.series.len().max(1) as f32); + let value = ScaleLinear::new( + self.points + .iter() + .flat_map(|d| d.values.iter().copied()) + .chain(Some(0.)), + [baseline, TOP_GAP], + ); + (band, value) + } +} + +impl IntoElement for GroupedBars { + type Element = PlotElement; + + fn into_element(self) -> Self::Element { + PlotElement::new(self) + } +} + +impl Plot for GroupedBars { + fn paint(&mut self, bounds: Bounds, window: &mut Window, cx: &mut App) { + let (band_scale, value_scale) = self.scales(bounds); + let band_width = band_scale.band_width(); + let height = bounds.size.height.as_f32(); + let baseline = height - axis_gap(); + let zero = value_scale.tick(&0.).unwrap_or(baseline); + let palette = palette(cx); + let n = self.series.len(); + + // The axis line sits at zero, which is mid-plot when the data crosses + // it; the band labels stay at the bottom, clear of any bar. + PlotAxis::new() + .stroke(cx.theme().border) + .x(px(zero)) + .paint(&bounds, window, cx); + let shown = labeled(self.points.len(), self.label_count); + let labels = self + .points + .iter() + .enumerate() + .filter(|(i, _)| shown[*i]) + .filter_map(|(_, d)| { + let tick = band_scale.tick(&d.band)?; + Some( + Text::new( + d.band.clone(), + point(px(tick + band_width / 2.), px(baseline + TEXT_GAP)), + cx.theme().muted_foreground, + ) + .align(TextAlign::Center), + ) + }) + .collect(); + PlotLabel::new(labels).paint(&bounds, window, cx); + + // The grid skips the baseline, which the axis line already draws. + let ticks = value_ticks(TOP_GAP, baseline, TICK_COUNT); + Grid::new() + .y(ticks[..ticks.len() - 1].to_vec()) + .stroke(cx.theme().chart_grid) + .dash_array(&[px(4.), px(2.)]) + .paint(&bounds, window); + + // Only the cells the query returned: a series a band lacks draws no + // bar there, rather than a zero the data never stated. + let mut bars = Vec::new(); + for d in &self.points { + for s in 0..n { + if d.present.get(s).copied().unwrap_or(false) { + bars.push((d.clone(), s)); + } + } + } + let (bar_width, _) = group_slot(band_width, n, 0); + Bar::new() + .data(bars) + .band_width(bar_width) + .cross(move |d: &(Arc, usize)| { + band_scale + .tick(&d.0.band) + .map(|tick| tick + group_slot(band_width, n, d.1).1) + }) + .base(move |_| zero) + .value(move |d: &(Arc, usize)| value_scale.tick(&d.0.values[d.1])) + .fill(move |d: &(Arc, usize), _, _| palette[d.1 % palette.len()]) + .paint(&bounds, window, cx); + } + + fn id(&self) -> Option { + Some(self.id.clone()) + } + + fn tooltip_state( + &self, + position: Point, + bounds: Bounds, + _cx: &App, + ) -> Option { + // The axis labels below the baseline are not a band. + let baseline = bounds.size.height.as_f32() - axis_gap(); + if position.y.as_f32() > baseline { + return None; + } + let (band_scale, _) = self.scales(bounds); + let index = band_scale.nearest_index(position.x.as_f32()); + let d = self.points.get(index)?; + let center = band_scale.tick(&d.band)? + band_scale.band_width() / 2.; + Some(TooltipState::new(index, point(px(center), position.y), vec![])) + } + + fn tooltip( + &self, + state: &TooltipState, + cursor: Point, + bounds: Bounds, + _window: &mut Window, + cx: &mut App, + ) -> Option { + let d = self.points.get(state.index)?; + let (band_scale, _) = self.scales(bounds); + let baseline = bounds.size.height.as_f32() - axis_gap(); + let palette = palette(cx); + + // The hovered band highlights whole, the way the catalog's bar chart + // highlights its bar; the tooltip lists the series the band has. + let mut tooltip = Tooltip::new(cursor, bounds.size) + .gap(px(8.)) + .cross_line( + CrossLine::new(state.cross_line) + .span(0., baseline) + .band(px(band_scale.band_width())), + ) + .title(d.band.clone()); + for (s, name) in self.series.iter().enumerate() { + if d.present.get(s).copied().unwrap_or(false) { + tooltip = tooltip.row( + palette[s % palette.len()], + name.clone(), + format_value(d.values[s]), + ); + } + } + Some(tooltip.into_any_element()) + } +} + +/// Whether a series' points are connected. `prepare` hands bands, not +/// numbers, so the x axis is categorical either way: a `scatter` spreads its +/// points across the same point scale a `line` uses, honestly unconnected. +#[derive(Clone, Copy, PartialEq, Eq)] +enum SeriesConnect { + Line, + Scatter, +} + +/// A `line` or `scatter` plot over one or more series, on shared scales: one +/// stroke per series, or one dot per point. Lines are tessellated through the +/// window's path caches, as the catalog's are, so a repaint while the +/// dashboard scrolls costs the quads, not the curves. +pub(crate) struct SeriesPlot { + id: ElementId, + connect: SeriesConnect, + points: Vec>, + series: Vec, + label_count: usize, +} + +impl SeriesPlot { + fn new(id: impl Into, plot: &PreparedPlot, connect: SeriesConnect) -> Self { + Self { + id: id.into(), + connect, + points: plot.points.clone(), + series: plot + .series_names + .iter() + .map(SharedString::from) + .collect(), + label_count: x_label_count(&plot.points), + } + } + + pub(crate) fn lines(id: impl Into, plot: &PreparedPlot) -> Self { + Self::new(id, plot, SeriesConnect::Line) + } + + pub(crate) fn scatter(id: impl Into, plot: &PreparedPlot) -> Self { + Self::new(id, plot, SeriesConnect::Scatter) + } + + /// The point (x) and value (y) scales for `bounds`, shared by `paint` and + /// the tooltip so the series and the hover dots stay aligned. The value + /// scale fits the data from zero, as the catalog's point charts do. + fn scales(&self, bounds: Bounds) -> (ScalePoint, ScaleLinear) { + let height = bounds.size.height.as_f32() - axis_gap(); + let x = ScalePoint::new( + self.points.iter().map(|d| d.band.clone()), + [0., bounds.size.width.as_f32()], + ); + let y = ScaleLinear::new( + self.points + .iter() + .flat_map(|d| { + d.values + .iter() + .zip(&d.present) + .filter_map(|(value, present)| present.then_some(*value)) + }) + .chain(Some(0.)), + [height, TOP_GAP], + ); + (x, y) + } + + /// Which series the hovered band has, in series order: the dots + /// `tooltip_state` collected are in the same order, so the two line up. + fn present_series(&self, index: usize) -> impl Iterator { + let d = self.points.get(index); + self.series + .iter() + .enumerate() + .filter(move |(s, _)| d.is_some_and(|d| d.present.get(*s).copied().unwrap_or(false))) + } +} + +impl IntoElement for SeriesPlot { + type Element = PlotElement; + + fn into_element(self) -> Self::Element { + PlotElement::new(self) + } +} + +impl Plot for SeriesPlot { + fn paint(&mut self, bounds: Bounds, window: &mut Window, cx: &mut App) { + let (x, y) = self.scales(bounds); + let height = bounds.size.height.as_f32() - axis_gap(); + let palette = palette(cx); + + let shown = labeled(self.points.len(), self.label_count); + let labels = self + .points + .iter() + .enumerate() + .filter(|(i, _)| shown[*i]) + .filter_map(|(i, d)| { + let tick = x.tick_at(i)?; + Some( + AxisText::new(d.band.clone(), px(tick), cx.theme().muted_foreground) + .align(point_label_align(i, self.points.len())), + ) + }); + PlotAxis::new() + .stroke(cx.theme().border) + .x(px(height)) + .x_label(labels) + .paint(&bounds, window, cx); + + let ticks = value_ticks(0., height, TICK_COUNT); + Grid::new() + .y(ticks[..ticks.len() - 1].to_vec()) + .stroke(cx.theme().chart_grid) + .dash_array(&[px(4.), px(2.)]) + .paint(&bounds, window); + + match self.connect { + SeriesConnect::Line => { + // A cell the query did not return is skipped, not drawn as + // zero; the line bridges the gap, as the catalog's line does + // over filtered data. + let caches = PathCaches::for_paint("spec-lines", window, cx); + caches.update(cx, |caches, _| { + for (s, _) in self.series.iter().enumerate() { + let (x, y) = (x.clone(), y.clone()); + let line = Line::new() + .data(self.points.iter().enumerate()) + .x(move |(i, _)| x.tick_at(*i)) + .y(move |(_, d)| { + d.present + .get(s) + .copied() + .unwrap_or(false) + .then(|| y.tick(&d.values[s])) + .flatten() + }) + .stroke(palette[s % palette.len()]) + .stroke_width(2.); + line.paint_cached(&bounds, caches.slot(s), window); + } + }); + } + SeriesConnect::Scatter => { + // Dots and nothing else: the quads `Line` would paint, without + // the path that would claim the points are connected. + for (i, d) in self.points.iter().enumerate() { + let Some(x_tick) = x.tick_at(i) else { continue }; + for s in 0..self.series.len() { + if !d.present.get(s).copied().unwrap_or(false) { + continue; + } + let Some(y_tick) = y.tick(&d.values[s]) else { continue }; + let color = palette[s % palette.len()]; + let origin = bounds.origin + + point(px(x_tick - DOT_SIZE / 2.), px(y_tick - DOT_SIZE / 2.)); + window.paint_quad(quad( + Bounds::new(origin, size(px(DOT_SIZE), px(DOT_SIZE))), + px(DOT_SIZE / 2.), + Background::from(color), + px(1.), + color, + BorderStyle::default(), + )); + } + } + } + } + } + + fn id(&self) -> Option { + Some(self.id.clone()) + } + + fn tooltip_state( + &self, + position: Point, + bounds: Bounds, + _cx: &App, + ) -> Option { + // The axis labels below the plot are not a datum. + let height = bounds.size.height.as_f32() - axis_gap(); + if position.y.as_f32() > height { + return None; + } + let (x, y) = self.scales(bounds); + let index = x.nearest_index(position.x.as_f32()); + let d = self.points.get(index)?; + let x_tick = x.tick_at(index)?; + // One dot per series the band has, in series order; `tooltip` colors + // them from the same ordering. + let dots = self + .present_series(index) + .filter_map(|(s, _)| Some(point(px(x_tick), px(y.tick(&d.values[s])?)))) + .collect(); + Some(TooltipState::new(index, point(px(x_tick), position.y), dots)) + } + + fn tooltip( + &self, + state: &TooltipState, + cursor: Point, + bounds: Bounds, + _window: &mut Window, + cx: &mut App, + ) -> Option { + let d = self.points.get(state.index)?; + let height = bounds.size.height.as_f32() - axis_gap(); + let palette = palette(cx); + let background = cx.theme().background; + + // The crosshair stays inside the plot; the dots mark the hovered + // band's points, one per series the band has. + let mut tooltip = Tooltip::new(cursor, bounds.size) + .gap(px(8.)) + .cross_line(CrossLine::new(state.cross_line).height(height)) + .dots( + state + .dots + .iter() + .zip(self.present_series(state.index)) + .map(|(p, (s, _))| { + Dot::new(*p) + .size(px(DOT_SIZE)) + .halo(px(HOVER_HALO)) + .stroke(background) + .fill(palette[s % palette.len()]) + }), + ) + .title(d.band.clone()); + for (s, name) in self.present_series(state.index) { + tooltip = tooltip.row( + palette[s % palette.len()], + name.clone(), + format_value(d.values[s]), + ); + } + Some(tooltip.into_any_element()) + } +} + +#[cfg(test)] +mod tests { + // Deliberately not `use super::*`: that pulls in `gpui_kit::*`, whose + // `test` macro shadows the built-in `#[test]`. + use super::{GROUP_GAP, group_slot, labeled, point_label_align, value_ticks, x_label_count}; + use crate::spec::prepare::PlotPoint; + use gpui_kit::{SharedString, TextAlign}; + use std::sync::Arc; + + #[test] + fn a_group_tiles_its_band() { + for series in 1..=12 { + let (width, first) = group_slot(90., series, 0); + assert_eq!(first, 0.); + let (_, last) = group_slot(90., series, series - 1); + // The last bar ends where the band ends. + assert!((last + width - 90.).abs() < 1e-4, "{series} series"); + if series > 1 { + let (_, second) = group_slot(90., series, 1); + assert!((second - width - GROUP_GAP).abs() < 1e-4, "{series} series"); + } + } + } + + #[test] + fn a_lone_bar_takes_the_band() { + assert_eq!(group_slot(48., 1, 0), (48., 0.)); + } + + #[test] + fn value_ticks_span_top_to_baseline() { + assert_eq!(value_ticks(0., 100., 5), vec![0., 25., 50., 75., 100.]); + assert_eq!(value_ticks(10., 110., 2), vec![10., 110.]); + } + + #[test] + fn edge_labels_hug_their_edges() { + assert_eq!(point_label_align(0, 5), TextAlign::Left); + assert_eq!(point_label_align(4, 5), TextAlign::Right); + assert_eq!(point_label_align(2, 5), TextAlign::Center); + assert_eq!(point_label_align(0, 1), TextAlign::Center); + } + + #[test] + fn labels_spread_from_first_to_last() { + assert_eq!(labeled(4, 2), vec![true, false, false, true]); + assert_eq!(labeled(5, 3), vec![true, false, true, false, true]); + assert!(labeled(3, 12).iter().all(|l| *l)); + assert_eq!(labeled(0, 6), Vec::::new()); + } + + #[test] + fn wide_labels_mean_fewer_ticks() { + let point = |band: &str| { + Arc::new(PlotPoint { + band: band.into(), + label: SharedString::default(), + values: vec![1.], + present: vec![true], + ix: 0, + }) + }; + // Ten-character dates get a handful of labels, short CJK bands more, + // and the count never drops below the two edge labels. + assert_eq!(x_label_count(&[point("2026-09-01")]), 5); + assert_eq!(x_label_count(&[point("手机银行")]), 10); + assert_eq!(x_label_count(&[point("")]), 12); + } +} diff --git a/src/spec/view.rs b/src/spec/view.rs index 6ce9e86..2fd635e 100644 --- a/src/spec/view.rs +++ b/src/spec/view.rs @@ -11,10 +11,11 @@ //! the app rule: a spec that no longer validates never replaces a working //! dashboard — the previous one stays up and the reason appears above it. //! -//! One chart note: only `AreaChart` in the catalog takes more than one series, -//! so a multi-series `line` reads as a faintly filled area and a multi-series -//! `scatter` as unmarked lines, while a multi-series `bar` is one small chart -//! per series, stacked and scrolled. +//! One chart note: the catalog's `BarChart` and `LineChart` draw a single +//! series and its one multi-series chart fills areas, so a pivoted `bar`, a +//! pivoted `line` and every `scatter` are drawn by the plots in `plot.rs` — +//! grouped bars, bare lines, unconnected dots — on the same primitives the +//! catalog charts compose. //! //! The toolbar's source toggle swaps the whole body for the spec's text in an //! editor — not a read-only one: the source is where a dashboard is fixed. @@ -54,9 +55,11 @@ use crate::i18n::{tr, trf}; use crate::query::{ColumnKind, QueryOutcome, QueryResult}; use crate::spec::complete::{self, CompletionKind}; use crate::spec::model::{self, Spec}; -use crate::spec::prepare::{prepare, PlotPoint, PreparedPlot}; +use crate::spec::plot::{GroupedBars, SeriesPlot, x_label_count}; use crate::ui::chart::format_value; +use crate::spec::prepare::{prepare, PlotPoint, PreparedPlot}; use crate::ui::completion::starts_with_ignore_case; +use crate::ui::results::fit_column_width; /// The plot panels' default and drag bounds, in the spirit of the workspace's /// own results split. @@ -755,7 +758,13 @@ fn chart_element(plot_ix: usize, plot: &PreparedPlot, cx: &App) -> AnyElement { cx.theme().chart_5, ]; let id = ("dashboard-chart", plot_ix); - let tick_margin = (plot.points.len() / 10).max(1); + // The hand-built plots key their hover state and path caches on the + // plot's name — unique after validation — which survives reordering the + // spec's blocks where the panel index would not. + let named_id = || (ElementId::from("dashboard-chart"), plot.name.clone()); + // A label count the axis can fit, not a stride: wide labels (dates) get + // few ticks, and the first and last value are always among them. + let x_labels = x_label_count(&plot.points); let single = plot.series_names.len() <= 1; let name = plot .series_names @@ -769,53 +778,14 @@ fn chart_element(plot_ix: usize, plot: &PreparedPlot, cx: &App) -> AnyElement { .value(|d: &Arc| d.values[0]) .fill(move |d: &Arc, _, _, _| palette[d.ix % palette.len()]) .label(|d: &Arc| d.label.clone()) + .tooltip_value(|d: &Arc, _| d.label.clone()) + .band_tick_count(x_labels) .id(id) .name(name) .into_any_element(), - // A band scale cannot group bars side by side, so each series draws - // its own chart, one under the next. - "bar" => { - let mut stack = v_flex() - .id(format!("dashboard-bars-{}", plot.name)) - .size_full() - .overflow_y_scroll() - .gap_4() - .p_2(); - for (s, series_name) in plot.series_names.iter().enumerate() { - let color = palette[s % palette.len()]; - let data: Vec> = plot - .points - .iter() - .filter(|d| d.present[s]) - .cloned() - .collect(); - stack = stack.child( - v_flex() - .flex_none() - .gap_1() - .child( - div() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child(series_name.clone()), - ) - .child( - div().h(px(240.)).flex_none().child( - BarChart::new(data) - .band(|d: &Arc| d.band.clone()) - .value(move |d: &Arc| d.values[s]) - .fill(move |_: &Arc, _, _, _| color) - .label(move |d: &Arc| { - SharedString::from(format_value(d.values[s])) - }) - .id(format!("dashboard-chart-{}-{s}", plot.name)) - .name(series_name.clone()), - ), - ), - ); - } - stack.into_any_element() - } + // A band scale cannot group a single-series BarChart's bars, so a + // pivoted bar draws on the hand-built grouped chart instead. + "bar" => GroupedBars::new(named_id(), plot).into_any_element(), "area" if single => { let color = palette[0]; AreaChart::new(plot.points.clone()) @@ -829,53 +799,47 @@ fn chart_element(plot_ix: usize, plot: &PreparedPlot, cx: &App) -> AnyElement { linear_color_stop(color.opacity(0.05), 0.), )) .name(name) - .tick_margin(tick_margin) + .x_tick_count(x_labels) + .tooltip_value(|_: &Arc, _: usize, v: f64| format_value(v).into()) .into_any_element() } - "scatter" if single => LineChart::new(plot.points.clone()) - .x(|d: &Arc| d.band.clone()) - .y(|d: &Arc| d.values[0]) - .dot() - .stroke(palette[0]) - .name(name) - .id(id) - .tick_margin(tick_margin) - .into_any_element(), + // A scatter's points are not connected, and its x is the band axis + // `prepare` built — the hand-built plot draws it that way, one series + // or many. + "scatter" => SeriesPlot::scatter(named_id(), plot).into_any_element(), _ if single => LineChart::new(plot.points.clone()) .x(|d: &Arc| d.band.clone()) .y(|d: &Arc| d.values[0]) .stroke(palette[0]) .name(name) .id(id) - .tick_margin(tick_margin) + .x_tick_count(x_labels) + .tooltip_value(|_: &Arc, v: f64| format_value(v).into()) .into_any_element(), - // Pivoted line/area/scatter all draw on the catalog's one multi-series - // chart; how much fill distinguishes them. - _ => { + // The catalog's one multi-series chart fills; filling is what `area` + // asks for. + "area" => { let mut chart = AreaChart::new(plot.points.clone()) .x(|d: &Arc| d.band.clone()) .id(id) - .tick_margin(tick_margin); + .x_tick_count(x_labels) + .tooltip_value(|_: &Arc, _: usize, v: f64| format_value(v).into()); for (s, series_name) in plot.series_names.iter().enumerate() { let color = palette[s % palette.len()]; - let (top, bottom) = match plot.kind.as_str() { - "area" => (0.4, 0.05), - "line" => (0.10, 0.02), - // A scatter's fill would claim a density the points lack. - _ => (0.0, 0.0), - }; chart = chart .y(move |d: &Arc| d.values[s]) .stroke(color) .fill(linear_gradient( 0., - linear_color_stop(color.opacity(top), 1.), - linear_color_stop(color.opacity(bottom), 0.), + linear_color_stop(color.opacity(0.4), 1.), + linear_color_stop(color.opacity(0.05), 0.), )) .name(series_name.clone()); } chart.into_any_element() } + // A pivoted line draws lines, no fill. + _ => SeriesPlot::lines(named_id(), plot).into_any_element(), } } @@ -927,7 +891,8 @@ impl SpecTableDelegate { .iter() .enumerate() .map(|(ix, column)| { - let mut spec = Column::new(format!("c{ix}"), column.name.clone()); + let width = fit_column_width(&column.name, &result.rows, ix, 0.); + let mut spec = Column::new(format!("c{ix}"), column.name.clone()).width(width); if column.kind == ColumnKind::Numeric { spec = spec.text_right(); } diff --git a/src/ui/results.rs b/src/ui/results.rs index dd4dfba..79be93f 100644 --- a/src/ui/results.rs +++ b/src/ui/results.rs @@ -13,6 +13,7 @@ use gpui_kit::component::label::Label; use gpui_kit::component::notification::Notification; use gpui_kit::component::tab::{Tab, TabBar}; use gpui_kit::component::table::{Column, DataTable, TableDelegate, TableState}; +use gpui_kit::component::toolbar::{Toolbar, ToolbarGroup}; use gpui_kit::component::{ h_flex, v_flex, ActiveTheme, Disableable, Icon, IconName, Sizable, StyledExt, WindowExt, }; @@ -52,6 +53,9 @@ const LEADING_COLUMNS: usize = 1; /// Per-cell copy buttons build their ElementId as `row * MAX_ID_COLUMNS + /// col`, so a result set is assumed to never exceed this many columns. const MAX_ID_COLUMNS: usize = 10_000; +/// Room the per-cell copy button takes beside the text; it is laid out even +/// while hidden, so a fitted column must leave space for it. +const COPY_BUTTON_WIDTH: f32 = 24.; /// Compact cell padding shared by header and body cells. fn cell_paddings() -> Edges { @@ -63,6 +67,44 @@ fn cell_paddings() -> Edges { } } +/// Advance of one narrow character in the table's monospace cell font; a +/// wide (CJK, fullwidth) character takes two. An estimate made without the +/// text system, so it errs a little wide rather than clip. +const CELL_CHAR_WIDTH: f32 = 9.6; +/// A fitted column is never narrower than this, so short values and +/// one-letter headers still leave room to grab the resize handle. +const MIN_FIT_WIDTH: f32 = 80.; +/// Nor wider than this: one long cell must not push every other column off +/// screen. The rest ellipsizes, and the column can still be dragged wider. +const MAX_FIT_WIDTH: f32 = 360.; +/// Rows sampled to fit a column. A result set can hold 100,000 rows; the +/// first few hundred are what the user sees first. +const FIT_SAMPLE_ROWS: usize = 200; + +/// Width for column `col` of `rows` that shows its header and sampled cells +/// unclipped, within [`MIN_FIT_WIDTH`, `MAX_FIT_WIDTH`]. `extra` is room the +/// cell spends on something besides its text, such as a copy button. Columns +/// that together exceed the viewport scroll horizontally. +pub(crate) fn fit_column_width(header: &str, rows: &[Vec], col: usize, extra: f32) -> f32 { + let text_cols = rows + .iter() + .take(FIT_SAMPLE_ROWS) + .filter_map(|row| row.get(col)) + .map(|cell| display_columns(cell)) + .chain(std::iter::once(display_columns(header))) + .max() + .unwrap_or(0); + let paddings = cell_paddings(); + let chrome = f32::from(paddings.left + paddings.right) + extra; + (text_cols as f32 * CELL_CHAR_WIDTH + chrome).clamp(MIN_FIT_WIDTH, MAX_FIT_WIDTH) +} + +/// Monospace columns `text` occupies on its widest line. +fn display_columns(text: &str) -> usize { + use unicode_width::UnicodeWidthStr; + text.lines().map(UnicodeWidthStr::width).max().unwrap_or(0) +} + /// Visible-row index map for `filter` over `rows`: the source index of every /// row containing the (case-insensitive) filter text in any cell. `None` /// means unfiltered — callers then use the row index directly, so an empty @@ -148,7 +190,8 @@ impl ResultTableDelegate { let mut columns = Vec::with_capacity(result.columns.len() + LEADING_COLUMNS); columns.push(Self::index_column()); columns.extend(result.columns.iter().enumerate().map(|(ix, column)| { - let mut spec = Column::new(format!("c{ix}"), column.name.clone()); + let width = fit_column_width(&column.name, &result.rows, ix, COPY_BUTTON_WIDTH); + let mut spec = Column::new(format!("c{ix}"), column.name.clone()).width(width); if column.kind == ColumnKind::Numeric { spec = spec.text_right(); } @@ -619,29 +662,31 @@ impl ResultsPanel { ) }) .child( - h_flex() - .gap_2() - .child( - Button::new("export-csv") - .outline() - .xsmall() - .icon(gpui_kit::assets::IconName::Download) - .label(tr("results.export_csv")) - .disabled(!has_rows) - .on_click(cx.listener(|this, _, window, cx| { - this.open_export_dialog(ExportFormat::Csv, window, cx); - })), - ) + // A real toolbar, not an h_flex: roving arrow-key focus and + // the compact ghost treatment come with it. + Toolbar::new("results-export-toolbar") + .xsmall() .child( - Button::new("export-parquet") - .outline() - .xsmall() - .icon(gpui_kit::assets::IconName::Download) - .label(tr("results.export_parquet")) - .disabled(!has_rows) - .on_click(cx.listener(|this, _, window, cx| { - this.open_export_dialog(ExportFormat::Parquet, window, cx); - })), + ToolbarGroup::new("results-export-group") + .gap_1() + .child( + Button::new("export-csv") + .icon(gpui_kit::assets::IconName::Download) + .label(tr("results.export_csv")) + .disabled(!has_rows) + .on_click(cx.listener(|this, _, window, cx| { + this.open_export_dialog(ExportFormat::Csv, window, cx); + })), + ) + .child( + Button::new("export-parquet") + .icon(gpui_kit::assets::IconName::Download) + .label(tr("results.export_parquet")) + .disabled(!has_rows) + .on_click(cx.listener(|this, _, window, cx| { + this.open_export_dialog(ExportFormat::Parquet, window, cx); + })), + ), ), ) } @@ -894,7 +939,7 @@ fn format_thousands(n: usize) -> String { #[cfg(test)] mod tests { - use super::filter_row_indices; + use super::{filter_row_indices, fit_column_width, MAX_FIT_WIDTH, MIN_FIT_WIDTH}; fn rows() -> Vec> { vec![ @@ -926,4 +971,24 @@ mod tests { let mapped = filter_row_indices(&rows(), "2026").unwrap(); assert_eq!(mapped, vec![0, 1, 2]); } + + #[test] + fn fitted_width_grows_with_content_and_counts_wide_characters_double() { + let rows = rows(); + let date = fit_column_width("date", &rows, 0, 0.); + let category = fit_column_width("category", &rows, 1, 0.); + // "2026-09-09" is 10 narrow columns; "数码配件" is 4 wide ones, 8 columns. + assert!(date > MIN_FIT_WIDTH, "{date}"); + assert!(category < date, "{category} vs {date}"); + assert_eq!(fit_column_width("a", &rows, 1, 30.), category + 30.); + } + + #[test] + fn fitted_width_stays_within_bounds() { + let rows = vec![vec!["x".into(), "y".repeat(500)]]; + assert_eq!(fit_column_width("n", &rows, 0, 0.), MIN_FIT_WIDTH); + assert_eq!(fit_column_width("n", &rows, 1, 0.), MAX_FIT_WIDTH); + // The header alone can widen a column of short values. + assert!(fit_column_width("a_rather_long_header", &rows, 0, 0.) > MIN_FIT_WIDTH); + } } diff --git a/src/ui/title_bar.rs b/src/ui/title_bar.rs index 684e8da..6c8a820 100644 --- a/src/ui/title_bar.rs +++ b/src/ui/title_bar.rs @@ -6,6 +6,7 @@ use gpui_kit::component::dialog::DialogFooter; use gpui_kit::component::input::{Input, InputContentType, InputState}; use gpui_kit::component::menu::{DropdownMenu, PopupMenuItem}; use gpui_kit::component::notification::Notification; +use gpui_kit::component::toolbar::{Toolbar, ToolbarGroup}; use gpui_kit::component::{ h_flex, v_flex, ActiveTheme, Disableable, IconName, Sizable, Theme, ThemeMode, TitleBar, WindowExt, @@ -355,23 +356,19 @@ impl Render for TitleBarView { let dark = cx.theme().mode.is_dark(); TitleBar::new().child( - h_flex() + // One toolbar across the whole bar: it owns roving arrow-key + // focus and the compact ghost look of every hosted button. + Toolbar::new("title-bar-toolbar") .w_full() - .items_center() - .gap_2() + .xsmall() .child( - h_flex() - .flex_1() - .min_w_0() - .justify_start() - .gap_2() + ToolbarGroup::new("title-bar-sources") + .gap_1() // With the sidebar put away, the way back sits where // the sidebar would begin. .when(sidebar_collapsed, |this| { this.child( Button::new("expand-sidebar") - .ghost() - .xsmall() .icon(IconName::PanelLeftOpen) .tooltip_with_action( tr("sidebar.expand"), @@ -386,8 +383,6 @@ impl Render for TitleBarView { }) .child( Button::new("open-data") - .ghost() - .xsmall() .icon(IconName::FolderOpen) .label(tr("title_bar.open_data")) .loading(opening) @@ -396,15 +391,14 @@ impl Render for TitleBarView { ) .child( Button::new("configure-s3") - .ghost() - .xsmall() .icon(IconName::Globe) .label("S3") .tooltip(tr("title_bar.configure_s3")) .on_click(cx.listener(Self::open_s3_dialog)), ), ) - .child( + .content(div().flex_1()) + .content( h_flex() .min_w_0() .gap_2() @@ -424,16 +418,12 @@ impl Render for TitleBarView { ) }), ) + .content(div().flex_1()) .child( - h_flex() - .flex_1() - .min_w_0() - .justify_end() - .gap_2() + ToolbarGroup::new("title-bar-window") + .gap_1() .child( Button::new("setup") - .ghost() - .xsmall() .icon(IconName::Bot) .tooltip(tr("setup.title")) .on_click(|_, window, cx| { @@ -442,8 +432,6 @@ impl Render for TitleBarView { ) .child( Button::new("toggle-language") - .ghost() - .xsmall() .label(match crate::i18n::current() { Language::Zh => "EN", Language::En => "中", @@ -451,9 +439,13 @@ impl Render for TitleBarView { .tooltip(tr("title_bar.toggle_language")) .on_click(Self::toggle_language), ) - .child( + // `dropdown_menu` wraps the Button in a popover that is + // not `Sizable`, so this trigger goes in as content and + // carries the toolbar's ghost/compact look by hand. + .content( Button::new("ui-size") .ghost() + .compact() .xsmall() .icon(IconName::ALargeSmall) .tooltip(tr("title_bar.ui_size")) @@ -475,8 +467,6 @@ impl Render for TitleBarView { ) .child( Button::new("toggle-theme") - .ghost() - .xsmall() .icon(if dark { IconName::Sun } else { IconName::Moon }) .tooltip(tr("title_bar.toggle_theme")) .on_click(Self::toggle_theme), From a6e22b1ee9fd1a60930025a92ce412d454049368 Mon Sep 17 00:00:00 2001 From: JetSquirrel Date: Mon, 28 Sep 2026 23:32:26 +0800 Subject: [PATCH 4/4] Give EmptyHeader its named slots in relation_panel EmptyHeader never accepted children: media, title and description are slot methods. The empty state failed to materialize and logged an error every frame the panel showed it. --- examples/relation_panel/main.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/relation_panel/main.js b/examples/relation_panel/main.js index 39169bd..3efa319 100644 --- a/examples/relation_panel/main.js +++ b/examples/relation_panel/main.js @@ -303,9 +303,9 @@ export default class RelationPanel extends View { .child( new Empty().child( new EmptyHeader() - .child(new EmptyMedia()) - .child(new EmptyTitle().child("This connection has no relations")) - .child( + .media(new EmptyMedia()) + .title(new EmptyTitle().child("This connection has no relations")) + .description( new EmptyDescription().child( "Open a data file or database in the main window first; this app queries that connection.", ),