From 80ae015ed04dcec2c49c782c27a857acd0156d7f Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Tue, 9 Jun 2026 07:36:38 +0200 Subject: [PATCH 1/2] Recent: Include filter and charts colors * Include filters and charts colors in recent sessions while keeping backward compatibility with the current recent session to avoid breaking them for current alpha users * Apply colors for converting from legacy recent sessions to the new ones --- crates/app/src/host/common/colors.rs | 40 +++ crates/app/src/host/service/presets_io/v2.rs | 40 +-- .../service/storage/recent/legacy/history.rs | 121 +++++++- .../src/host/service/storage/recent/mod.rs | 29 +- .../src/host/ui/registry/presets/capture.rs | 18 +- .../app/src/host/ui/storage/recent/session.rs | 107 +++++++- .../session/ui/bottom_panel/library/mod.rs | 6 +- .../session/ui/bottom_panel/presets/apply.rs | 30 +- crates/app/src/session/ui/recent.rs | 199 ++++++++++++-- crates/app/src/session/ui/shared/mod.rs | 103 ++++--- .../session/ui/shared/searching/filters.rs | 259 ++++++++++-------- .../app/src/session/ui/side_panel/filters.rs | 74 +++-- 12 files changed, 747 insertions(+), 279 deletions(-) diff --git a/crates/app/src/host/common/colors.rs b/crates/app/src/host/common/colors.rs index 8605972ec2..603b4db561 100644 --- a/crates/app/src/host/common/colors.rs +++ b/crates/app/src/host/common/colors.rs @@ -1,4 +1,7 @@ +//! Shared color palettes, color pairs, and color serialization helpers. + use egui::{Color32, Stroke}; +use serde::{Deserialize, Serialize}; const MAIN_ACCENT_BACKGROUND_DARK: Color32 = Color32::from_rgb(36, 44, 58); const MAIN_ACCENT_BACKGROUND_LIGHT: Color32 = Color32::from_rgb(220, 228, 240); @@ -103,6 +106,43 @@ impl ColorPair { } } +/// Color bytes stored for serialization in unpremultiplied sRGBA order. +pub type StoredRgba = [u8; 4]; + +/// Foreground/background color pair stored for serialization. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct StoredColorPair { + /// Foreground color. + pub fg: StoredRgba, + /// Background color. + pub bg: StoredRgba, +} + +impl From for StoredColorPair { + fn from(value: ColorPair) -> Self { + Self { + fg: color_to_rgba(value.fg), + bg: color_to_rgba(value.bg), + } + } +} + +impl From for ColorPair { + fn from(value: StoredColorPair) -> Self { + Self::new(color_from_rgba(value.fg), color_from_rgba(value.bg)) + } +} + +/// Converts an egui color into stored unpremultiplied sRGBA bytes. +pub fn color_to_rgba(color: Color32) -> StoredRgba { + color.to_srgba_unmultiplied() +} + +/// Converts stored unpremultiplied sRGBA bytes into an egui color. +pub fn color_from_rgba([r, g, b, a]: StoredRgba) -> Color32 { + Color32::from_rgba_unmultiplied(r, g, b, a) +} + /// Foreground/background colors used for temporary search highlighting. pub const TEMP_SEARCH_COLORS: ColorPair = ColorPair::new( Color32::from_rgb(230, 234, 244), diff --git a/crates/app/src/host/service/presets_io/v2.rs b/crates/app/src/host/service/presets_io/v2.rs index 6aab73b6da..dda0957638 100644 --- a/crates/app/src/host/service/presets_io/v2.rs +++ b/crates/app/src/host/service/presets_io/v2.rs @@ -3,14 +3,13 @@ //! Version 2 documents store named filter and search-value row snapshots, //! including enabled state and colors. -use egui::Color32; use processor::search::filter::SearchFilter; use serde::{Deserialize, Serialize}; use serde_json::Value; use uuid::Uuid; use crate::host::{ - common::colors::ColorPair, + common::colors::{ColorPair, StoredColorPair, StoredRgba, color_from_rgba, color_to_rgba}, ui::registry::presets::{Preset, PresetFilterEntry, PresetSearchValueEntry}, }; @@ -19,8 +18,6 @@ use super::{ validate_search_value_entry, }; -type Rgba = [u8; 4]; - /// Preset payload stored in the current preset document. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] struct DocumentPreset { @@ -34,7 +31,7 @@ struct DocumentPreset { struct DocumentFilterEntry { filter: SearchFilter, enabled: bool, - colors: DocumentColorPair, + colors: StoredColorPair, } /// Chart/search-value row payload stored in the current preset document. @@ -42,13 +39,7 @@ struct DocumentFilterEntry { struct DocumentSearchValueEntry { filter: SearchFilter, enabled: bool, - color: Rgba, -} - -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -struct DocumentColorPair { - fg: Rgba, - bg: Rgba, + color: StoredRgba, } /// Versioned preset document stored on disk. @@ -143,7 +134,7 @@ impl From for DocumentFilterEntry { Self { filter: value.filter, enabled: value.enabled, - colors: DocumentColorPair::from(value.colors), + colors: StoredColorPair::from(value.colors), } } } @@ -158,15 +149,6 @@ impl From for DocumentSearchValueEntry { } } -impl From for DocumentColorPair { - fn from(value: ColorPair) -> Self { - Self { - fg: color_to_rgba(value.fg), - bg: color_to_rgba(value.bg), - } - } -} - impl From for Preset { fn from(value: DocumentPreset) -> Self { Self { @@ -200,20 +182,6 @@ impl From for PresetSearchValueEntry { } } -impl From for ColorPair { - fn from(value: DocumentColorPair) -> Self { - Self::new(color_from_rgba(value.fg), color_from_rgba(value.bg)) - } -} - -fn color_to_rgba(color: Color32) -> Rgba { - color.to_srgba_unmultiplied() -} - -fn color_from_rgba([r, g, b, a]: Rgba) -> Color32 { - Color32::from_rgba_unmultiplied(r, g, b, a) -} - #[cfg(test)] mod tests { use egui::Color32; diff --git a/crates/app/src/host/service/storage/recent/legacy/history.rs b/crates/app/src/host/service/storage/recent/legacy/history.rs index 0c350af181..68cd69915c 100644 --- a/crates/app/src/host/service/storage/recent/legacy/history.rs +++ b/crates/app/src/host/service/storage/recent/legacy/history.rs @@ -8,9 +8,13 @@ use serde_json::{Map, Value}; use processor::search::filter::SearchFilter; use crate::host::{ - common::parsers::ParserNames, + common::{ + colors::{StoredColorPair, StoredRgba}, + parsers::ParserNames, + }, ui::storage::recent::session::{ - RecentSessionSource, RecentSessionStateSnapshot, SearchFilterSnapshot, + RecentFilterSnapshot, RecentSearchValueSnapshot, RecentSessionSource, + RecentSessionStateSnapshot, }, }; @@ -280,7 +284,7 @@ fn parse_collection_entry_object( fn parse_filters( value: &Value, default_enabled: bool, - output: &mut Vec, + output: &mut Vec, ) -> usize { parse_one_or_many(value, |item| parse_filter(item, default_enabled), output) } @@ -289,7 +293,7 @@ fn parse_filters( fn parse_charts( value: &Value, default_enabled: bool, - output: &mut Vec, + output: &mut Vec, ) -> usize { parse_one_or_many(value, |item| parse_chart(item, default_enabled), output) } @@ -313,7 +317,7 @@ fn parse_one_or_many( } /// Parses one filter snapshot, returning `None` when filter text is missing. -fn parse_filter(value: &Value, default_enabled: bool) -> Option { +fn parse_filter(value: &Value, default_enabled: bool) -> Option { let filter_value = value.get("filter").unwrap_or(value); let text = if let Some(text) = filter_value.as_str() { text @@ -331,19 +335,23 @@ fn parse_filter(value: &Value, default_enabled: bool) -> Option Option { +fn parse_chart(value: &Value, default_enabled: bool) -> Option { let chart_value = value.get("chart").unwrap_or(value); let text = chart_value .get("filter") @@ -354,14 +362,68 @@ fn parse_chart(value: &Value, default_enabled: bool) -> Option Option { + let colors = value.get("colors")?.as_object()?; + let fg = colors.get("color")?.as_str()?; + let bg = colors.get("background")?.as_str()?; + + let color_pair = StoredColorPair { + fg: parse_legacy_hex_rgba(fg)?, + bg: parse_legacy_hex_rgba(bg)?, + }; + + Some(color_pair) +} + +fn parse_legacy_chart_color(value: &Value) -> Option { + let color = value.get("color")?.as_str()?; + parse_legacy_hex_rgba(color) +} + +/// Parses the only legacy color format this importer supports: `#RRGGBB`. +fn parse_legacy_hex_rgba(value: &str) -> Option { + let bytes = value.as_bytes(); + if bytes.len() != 7 || bytes[0] != b'#' { + return None; + } + + let red = parse_hex_byte(&bytes[1..3])?; + let green = parse_hex_byte(&bytes[3..5])?; + let blue = parse_hex_byte(&bytes[5..7])?; + + Some([red, green, blue, 255]) +} + +fn parse_hex_byte(pair: &[u8]) -> Option { + let [high, low] = pair else { + return None; + }; + let high = hex_value(*high)?; + let low = hex_value(*low)?; + + Some((high << 4) | low) +} + +fn hex_value(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, + } +} + /// Imports bookmark positions into `output` and returns the number of skipped entries. fn parse_bookmarks(value: &Value, output: &mut Vec) -> usize { parse_one_or_many( @@ -511,9 +573,10 @@ mod tests { relation_ids: HashSet::from([String::from("definition-id")]), last_used: 1, state: RecentSessionStateSnapshot { - filters: vec![SearchFilterSnapshot { + filters: vec![RecentFilterSnapshot { filter: SearchFilter::plain("old"), enabled: true, + colors: None, }], ..Default::default() }, @@ -523,8 +586,11 @@ mod tests { relation_ids: HashSet::from([String::from("definition-id")]), last_used: 2, state: parsed_state(json!({ - "filters": [{ "filter": { "filter": "level=(warn|error)", "reg": true, "word": true, "cases": true } }], - "charts": [{ "filter": "cpu=(\\d+)" }], + "filters": [{ + "filter": { "filter": "level=(warn|error)", "reg": true, "word": true, "cases": true }, + "colors": { "color": "#010203", "background": "#040506" } + }], + "charts": [{ "filter": "cpu=(\\d+)", "color": "#070809" }], "disabled": [ { "filter": { "filter": "disabled filter", "active": true } }, { "chart": { "filter": "disabled chart", "active": true } } @@ -550,6 +616,11 @@ mod tests { assert!(imported_filter.filter.is_word()); assert!(!imported_filter.filter.is_ignore_case()); assert!(imported_filter.enabled); + let expected_colors = StoredColorPair { + fg: [1, 2, 3, 255], + bg: [4, 5, 6, 255], + }; + assert_eq!(imported_filter.colors, Some(expected_colors)); let disabled_filter = state .filters .iter() @@ -566,6 +637,7 @@ mod tests { assert!(imported_chart.filter.is_regex()); assert!(imported_chart.filter.is_ignore_case()); assert!(imported_chart.enabled); + assert_eq!(imported_chart.color, Some([7, 8, 9, 255])); let disabled_chart = state .search_values .iter() @@ -575,6 +647,33 @@ mod tests { assert_eq!(state.bookmarks, vec![42]); } + #[test] + fn missing_or_invalid_colors_do_not_skip_entries() { + let state = parsed_state(json!({ + "filters": [ + { "filter": { "filter": "missing colors" } }, + { + "filter": { "filter": "invalid colors" }, + "colors": { "color": "#010203", "background": "not-a-color" } + } + ], + "charts": [ + { "filter": "missing=(\\d+)" }, + { "filter": "invalid=(\\d+)", "color": "#123" } + ], + })); + + assert_eq!(state.filters.len(), 2); + assert!(state.filters.iter().all(|filter| filter.colors.is_none())); + assert_eq!(state.search_values.len(), 2); + assert!( + state + .search_values + .iter() + .all(|chart| chart.color.is_none()) + ); + } + /// Verifies that stream history returns state with bookmarks removed. #[test] fn does_not_attach_bookmarks_to_stream_history() { diff --git a/crates/app/src/host/service/storage/recent/mod.rs b/crates/app/src/host/service/storage/recent/mod.rs index b490866221..500cf26c3e 100644 --- a/crates/app/src/host/service/storage/recent/mod.rs +++ b/crates/app/src/host/service/storage/recent/mod.rs @@ -323,12 +323,13 @@ mod tests { common::time::unix_timestamp_now, host::{ command::OpenRecentSessionParam, + common::colors::StoredColorPair, service::storage::{STORAGE_DIR, storage_path_from_home}, ui::storage::{ recent::{ session::{ - RecentSessionRegistration, RecentSessionReopenMode, RecentSessionSnapshot, - RecentSessionSource, SearchFilterSnapshot, + RecentFilterSnapshot, RecentSearchValueSnapshot, RecentSessionRegistration, + RecentSessionReopenMode, RecentSessionSnapshot, RecentSessionSource, }, storage::{MAX_RECENT_SESSIONS, RecentSessionsStorage}, }, @@ -640,9 +641,10 @@ mod tests { "reg": true, "word": true, "cases": true, - } + }, + "colors": { "color": "#010203", "background": "#040506" } }], - "charts": [{ "filter": "cpu=(\\d+)" }], + "charts": [{ "filter": "cpu=(\\d+)", "color": "#070809" }], "bookmark": [{ "position": 42 }], } }), @@ -660,12 +662,18 @@ mod tests { assert!(filter.filter.is_word()); assert!(!filter.filter.is_ignore_case()); assert!(filter.enabled); + let expected_colors = StoredColorPair { + fg: [1, 2, 3, 255], + bg: [4, 5, 6, 255], + }; + assert_eq!(filter.colors, Some(expected_colors)); assert_eq!(state.search_values.len(), 1); let chart = &state.search_values[0]; assert_eq!(chart.filter.value, "cpu=(\\d+)"); assert!(chart.filter.is_regex()); assert!(chart.filter.is_ignore_case()); assert!(chart.enabled); + assert_eq!(chart.color, Some([7, 8, 9, 255])); assert_eq!(state.bookmarks, vec![42]); } @@ -862,13 +870,19 @@ mod tests { fs::write(&config_path, "test").expect("config file should be written"); let mut storage = RecentSessionsStorage::default(); let mut snapshot = file_snapshot(config_path); - snapshot.state.filters = vec![SearchFilterSnapshot { + let colors = StoredColorPair { + fg: [1, 2, 3, 4], + bg: [5, 6, 7, 8], + }; + snapshot.state.filters = vec![RecentFilterSnapshot { filter: SearchFilter::plain("status=ok").ignore_case(true), enabled: false, + colors: Some(colors), }]; - snapshot.state.search_values = vec![SearchFilterSnapshot { + snapshot.state.search_values = vec![RecentSearchValueSnapshot { filter: SearchFilter::plain("cpu=(\\d+)").regex(true), enabled: true, + color: Some([9, 10, 11, 12]), }]; snapshot.state.bookmarks = vec![2, 9]; storage.register_session(snapshot); @@ -893,9 +907,10 @@ mod tests { #[test] fn restore_modes_split_state() { let mut snapshot = stream_snapshot("127.0.0.1:5555"); - snapshot.state.filters = vec![SearchFilterSnapshot { + snapshot.state.filters = vec![RecentFilterSnapshot { filter: SearchFilter::plain("status=ok"), enabled: true, + colors: None, }]; let restore_request = resolve_open_request(OpenRecentSessionParam { diff --git a/crates/app/src/host/ui/registry/presets/capture.rs b/crates/app/src/host/ui/registry/presets/capture.rs index 0217aaf77f..453659ec00 100644 --- a/crates/app/src/host/ui/registry/presets/capture.rs +++ b/crates/app/src/host/ui/registry/presets/capture.rs @@ -103,15 +103,21 @@ mod tests { shared .filters .apply_filter(&mut filters_registry, first_filter_id); - shared - .filters - .apply_filter_with_state(&mut filters_registry, second_filter_id, false); + shared.filters.apply_filter_with_state( + &mut filters_registry, + second_filter_id, + false, + None, + ); shared .filters .apply_search_value(&mut filters_registry, first_value_id); - shared - .filters - .apply_search_value_with_state(&mut filters_registry, second_value_id, false); + shared.filters.apply_search_value_with_state( + &mut filters_registry, + second_value_id, + false, + None, + ); let expected_filter_entries = shared.filters.filter_entries.clone(); let expected_search_value_entries = shared.filters.search_value_entries.clone(); diff --git a/crates/app/src/host/ui/storage/recent/session.rs b/crates/app/src/host/ui/storage/recent/session.rs index 29e8d2f5e3..24ae125b22 100644 --- a/crates/app/src/host/ui/storage/recent/session.rs +++ b/crates/app/src/host/ui/storage/recent/session.rs @@ -13,7 +13,11 @@ use processor::search::filter::SearchFilter; use stypes::{FileFormat, ObserveOptions, ObserveOrigin, ParserType, Transport}; use uuid::Uuid; -use crate::host::common::{parsers::ParserNames, sources::StreamNames}; +use crate::host::common::{ + colors::{StoredColorPair, StoredRgba}, + parsers::ParserNames, + sources::StreamNames, +}; use super::source_key; @@ -74,16 +78,36 @@ pub enum RecentSessionSource { /// Stored semantic state for reopening a session. #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] pub struct RecentSessionStateSnapshot { - pub filters: Vec, - pub search_values: Vec, + /// Applied filter rows restored when reopening the session. + pub filters: Vec, + /// Applied chart/search-value rows restored when reopening the session. + pub search_values: Vec, + /// Bookmarked row indexes restored when reopening the session. pub bookmarks: Vec, } -/// Stored semantic filter or search-value row. +/// Stored semantic filter row. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct SearchFilterSnapshot { +pub struct RecentFilterSnapshot { + /// Filter definition persisted without its runtime registry ID. pub filter: SearchFilter, + /// Whether the filter is applied when the session is restored. pub enabled: bool, + /// Optional presentation colors; missing values use current defaults. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub colors: Option, +} + +/// Stored semantic chart/search-value row. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct RecentSearchValueSnapshot { + /// Search-value definition persisted without its runtime registry ID. + pub filter: SearchFilter, + /// Whether the search value is applied when the session is restored. + pub enabled: bool, + /// Optional presentation color; missing values use current defaults. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub color: Option, } impl RecentSessionRegistration { @@ -453,16 +477,17 @@ fn supports_bookmarks(sources: &[RecentSessionSource]) -> bool { mod tests { use std::path::PathBuf; + use processor::search::filter::SearchFilter; use stypes::{ DltParserSettings, ObserveOptions, ObserveOrigin, ParserType, TCPTransportConfig, Transport, UDPTransportConfig, }; - use crate::common::time::unix_timestamp_now; + use crate::{common::time::unix_timestamp_now, host::common::colors::StoredColorPair}; use super::{ - RecentSessionRegistration, RecentSessionSnapshot, RecentSessionSource, - RecentSessionStateSnapshot, SearchFilterSnapshot, + RecentFilterSnapshot, RecentSearchValueSnapshot, RecentSessionRegistration, + RecentSessionSnapshot, RecentSessionSource, RecentSessionStateSnapshot, }; fn snapshot_from_observe_options(options: ObserveOptions) -> RecentSessionSnapshot { @@ -623,18 +648,21 @@ mod tests { fibex_metadata: None, }), RecentSessionStateSnapshot { - filters: vec![SearchFilterSnapshot { - filter: processor::search::filter::SearchFilter::plain("level=warn"), + filters: vec![RecentFilterSnapshot { + filter: SearchFilter::plain("level=warn"), enabled: true, + colors: None, }], search_values: vec![ - SearchFilterSnapshot { - filter: processor::search::filter::SearchFilter::plain("cpu"), + RecentSearchValueSnapshot { + filter: SearchFilter::plain("cpu"), enabled: true, + color: None, }, - SearchFilterSnapshot { - filter: processor::search::filter::SearchFilter::plain("mem"), + RecentSearchValueSnapshot { + filter: SearchFilter::plain("mem"), enabled: false, + color: None, }, ], bookmarks: vec![4, 9, 12], @@ -659,6 +687,57 @@ mod tests { assert!(snapshot.tooltip().contains("- Bookmarks: 3")); } + #[test] + fn state_deserializes_rows_without_colors() { + let value = serde_json::json!({ + "filters": [{ + "filter": SearchFilter::plain("level=warn"), + "enabled": true, + }], + "search_values": [{ + "filter": SearchFilter::plain("cpu"), + "enabled": false, + }], + "bookmarks": [3], + }); + + let state: RecentSessionStateSnapshot = + serde_json::from_value(value).expect("old recent state should load"); + + assert_eq!(state.filters.len(), 1); + assert!(state.filters[0].colors.is_none()); + assert_eq!(state.search_values.len(), 1); + assert!(state.search_values[0].color.is_none()); + assert_eq!(state.bookmarks, vec![3]); + } + + #[test] + fn state_round_trip_preserves_colors() { + let colors = StoredColorPair { + fg: [1, 2, 3, 4], + bg: [5, 6, 7, 8], + }; + let state = RecentSessionStateSnapshot { + filters: vec![RecentFilterSnapshot { + filter: SearchFilter::plain("level=warn"), + enabled: true, + colors: Some(colors), + }], + search_values: vec![RecentSearchValueSnapshot { + filter: SearchFilter::plain("cpu"), + enabled: false, + color: Some([9, 10, 11, 12]), + }], + bookmarks: vec![4], + }; + + let json = serde_json::to_string(&state).expect("state should serialize"); + let parsed: RecentSessionStateSnapshot = + serde_json::from_str(&json).expect("state should deserialize"); + + assert_eq!(parsed, state); + } + #[test] fn restore_rebuilds_ids() { let snapshot = snapshot_from_observe_options(ObserveOptions { diff --git a/crates/app/src/session/ui/bottom_panel/library/mod.rs b/crates/app/src/session/ui/bottom_panel/library/mod.rs index ac7158d008..4d4401465f 100644 --- a/crates/app/src/session/ui/bottom_panel/library/mod.rs +++ b/crates/app/src/session/ui/bottom_panel/library/mod.rs @@ -822,7 +822,7 @@ mod tests { let filter_id = add_filter_definition(&mut registry, "status=ok"); shared .filters - .apply_filter_with_state(&mut registry, filter_id, false); + .apply_filter_with_state(&mut registry, filter_id, false, None); library.toggle_filter_definition(&mut shared, &mut actions, &mut registry, filter_id); @@ -867,7 +867,7 @@ mod tests { let filter_id = add_filter_definition(&mut registry, "status=ok"); shared .filters - .apply_filter_with_state(&mut registry, filter_id, false); + .apply_filter_with_state(&mut registry, filter_id, false, None); assert!(library.delete_filter_definition( &mut shared, @@ -910,7 +910,7 @@ mod tests { let value_id = add_search_value_definition(&mut registry, "duration=(\\d+)"); shared .filters - .apply_search_value_with_state(&mut registry, value_id, false); + .apply_search_value_with_state(&mut registry, value_id, false, None); library.toggle_search_value_definition(&mut shared, &mut actions, &mut registry, value_id); diff --git a/crates/app/src/session/ui/bottom_panel/presets/apply.rs b/crates/app/src/session/ui/bottom_panel/presets/apply.rs index c80ff22f33..95c2e976a8 100644 --- a/crates/app/src/session/ui/bottom_panel/presets/apply.rs +++ b/crates/app/src/session/ui/bottom_panel/presets/apply.rs @@ -65,8 +65,12 @@ impl PresetsUI { colors, } = entry; let filter_id = registry.filters.add_filter(FilterDefinition::new(filter)); - changed |= - shared.set_filter_entry_state(&mut registry.filters, filter_id, enabled, colors); + changed |= shared.apply_filter_with_state( + &mut registry.filters, + filter_id, + enabled, + Some(colors), + ); } for entry in search_values { @@ -78,11 +82,11 @@ impl PresetsUI { let value_id = registry .filters .add_search_value(SearchValueDefinition::new(filter)); - changed |= shared.set_search_value_entry_state( + changed |= shared.apply_search_value_with_state( &mut registry.filters, value_id, enabled, - color, + Some(color), ); } @@ -293,8 +297,8 @@ mod tests { )], ); - shared.apply_filter_with_state(&mut registry.filters, filter_id, false); - shared.apply_search_value_with_state(&mut registry.filters, value_id, false); + shared.apply_filter_with_state(&mut registry.filters, filter_id, false, None); + shared.apply_search_value_with_state(&mut registry.filters, value_id, false, None); let outcome = presets.apply_preset(&mut shared, &mut actions, &mut registry, preset_id); @@ -340,17 +344,17 @@ mod tests { )], ); - shared.set_filter_entry_state( + shared.apply_filter_with_state( &mut registry.filters, existing_filter_id, true, - existing_filter_colors.clone(), + Some(existing_filter_colors.clone()), ); - shared.set_search_value_entry_state( + shared.apply_search_value_with_state( &mut registry.filters, existing_value_id, true, - existing_value_color, + Some(existing_value_color), ); let outcome = presets.apply_preset(&mut shared, &mut actions, &mut registry, preset_id); @@ -494,12 +498,12 @@ mod tests { )], ); - shared.set_filter_entry_state(&mut registry.filters, filter_id, true, filter_colors); - shared.set_search_value_entry_state( + shared.apply_filter_with_state(&mut registry.filters, filter_id, true, Some(filter_colors)); + shared.apply_search_value_with_state( &mut registry.filters, value_id, true, - search_value_color, + Some(search_value_color), ); let revision = shared.recent_revision(); diff --git a/crates/app/src/session/ui/recent.rs b/crates/app/src/session/ui/recent.rs index afb04cbdf2..779c059b6a 100644 --- a/crates/app/src/session/ui/recent.rs +++ b/crates/app/src/session/ui/recent.rs @@ -3,10 +3,15 @@ use std::sync::Arc; use tokio::sync::mpsc::Sender; use crate::{ - host::ui::{ - UiActions, - registry::filters::{FilterDefinition, FilterRegistry, SearchValueDefinition}, - storage::recent::session::{RecentSessionStateSnapshot, SearchFilterSnapshot}, + host::{ + common::colors::{color_from_rgba, color_to_rgba}, + ui::{ + UiActions, + registry::filters::{FilterDefinition, FilterRegistry, SearchValueDefinition}, + storage::recent::session::{ + RecentFilterSnapshot, RecentSearchValueSnapshot, RecentSessionStateSnapshot, + }, + }, }, session::command::SessionCommand, }; @@ -64,14 +69,16 @@ impl RecentSessionRuntime { let mut changed_filters = false; for snapshot in restore_state.filters { let filter_id = registry.add_filter(FilterDefinition::new(snapshot.filter)); - shared.apply_filter_with_state(registry, filter_id, snapshot.enabled); + let colors = snapshot.colors.map(Into::into); + shared.apply_filter_with_state(registry, filter_id, snapshot.enabled, colors); changed_filters = true; } let mut changed_search_values = false; for snapshot in restore_state.search_values { let value_id = registry.add_search_value(SearchValueDefinition::new(snapshot.filter)); - shared.apply_search_value_with_state(registry, value_id, snapshot.enabled); + let color = snapshot.color.map(color_from_rgba); + shared.apply_search_value_with_state(registry, value_id, snapshot.enabled, color); changed_search_values = true; } @@ -195,9 +202,10 @@ pub fn capture_state_snapshot( .filter_map(|item| { registry .get_filter(&item.id) - .map(|definition| SearchFilterSnapshot { + .map(|definition| RecentFilterSnapshot { filter: definition.filter.clone(), enabled: item.enabled, + colors: Some(item.colors.clone().into()), }) }) .collect(); @@ -209,9 +217,10 @@ pub fn capture_state_snapshot( .filter_map(|item| { registry .get_search_value(&item.id) - .map(|definition| SearchFilterSnapshot { + .map(|definition| RecentSearchValueSnapshot { filter: definition.filter.clone(), enabled: item.enabled, + color: Some(color_to_rgba(item.color)), }) }) .collect(); @@ -238,6 +247,7 @@ pub fn capture_state_snapshot( mod tests { use std::path::PathBuf; + use egui::Color32; use tokio::{runtime::Runtime, sync::mpsc}; use uuid::Uuid; @@ -245,7 +255,14 @@ mod tests { use stypes::{FileFormat, ObserveOrigin, TCPTransportConfig, Transport}; use crate::{ - host::{common::parsers::ParserNames, ui::UiActions}, + host::{ + common::{ + colors, + colors::{ColorPair, StoredColorPair, color_to_rgba}, + parsers::ParserNames, + }, + ui::UiActions, + }, session::{types::ObserveOperation, ui::SessionInfo}, }; @@ -287,38 +304,46 @@ mod tests { SearchFilter::plain("value-2").word(true), )); - shared.apply_filter_with_state(&mut registry, first_filter, true); - shared.apply_filter_with_state(&mut registry, second_filter, false); - shared.apply_search_value_with_state(&mut registry, first_value, false); - shared.apply_search_value_with_state(&mut registry, second_value, true); + shared.apply_filter_with_state(&mut registry, first_filter, true, None); + shared.apply_filter_with_state(&mut registry, second_filter, false, None); + shared.apply_search_value_with_state(&mut registry, first_value, false, None); + shared.apply_search_value_with_state(&mut registry, second_value, true, None); shared.insert_bookmark(9); shared.insert_bookmark(2); let state = file_runtime().capture_opened_state(&shared, ®istry); + let first_filter_colors = colors::FILTER_HIGHLIGHT_COLORS[0].clone().into(); + let second_filter_colors = colors::FILTER_HIGHLIGHT_COLORS[1].clone().into(); assert_eq!( state.filters, vec![ - SearchFilterSnapshot { + RecentFilterSnapshot { filter: SearchFilter::plain("first"), enabled: true, + colors: Some(first_filter_colors), }, - SearchFilterSnapshot { + RecentFilterSnapshot { filter: SearchFilter::plain("second").regex(true), enabled: false, + colors: Some(second_filter_colors), }, ] ); + let first_value_color = color_to_rgba(colors::search_value_color(0)); + let second_value_color = color_to_rgba(colors::search_value_color(1)); assert_eq!( state.search_values, vec![ - SearchFilterSnapshot { + RecentSearchValueSnapshot { filter: SearchFilter::plain("value-1").ignore_case(true), enabled: false, + color: Some(first_value_color), }, - SearchFilterSnapshot { + RecentSearchValueSnapshot { filter: SearchFilter::plain("value-2").word(true), enabled: true, + color: Some(second_value_color), }, ] ); @@ -327,6 +352,34 @@ mod tests { assert!(state.bookmarks.contains(&9)); } + #[test] + fn captures_custom_colors() { + let mut shared = new_shared(ObserveOrigin::File( + String::from("source"), + FileFormat::Text, + PathBuf::from("source.log"), + )); + let mut registry = FilterRegistry::default(); + let filter = SearchFilter::plain("first"); + let filter_id = registry.add_filter(FilterDefinition::new(filter)); + let search_value = SearchFilter::plain("value=(\\d+)"); + let value_id = registry.add_search_value(SearchValueDefinition::new(search_value)); + shared.apply_filter(&mut registry, filter_id); + shared.apply_search_value(&mut registry, value_id); + let filter_colors = ColorPair::new(Color32::from_rgb(1, 2, 3), Color32::from_rgb(4, 5, 6)); + let value_color = Color32::from_rgb(7, 8, 9); + shared.set_filter_colors(&filter_id, filter_colors.clone()); + shared.set_search_value_color(&value_id, value_color); + + let state = file_runtime().capture_opened_state(&shared, ®istry); + + assert_eq!(state.filters[0].colors, Some(filter_colors.into())); + assert_eq!( + state.search_values[0].color, + Some(color_to_rgba(value_color)) + ); + } + #[test] fn registration_sets_update_baseline() { let mut shared = new_shared(ObserveOrigin::File( @@ -411,6 +464,112 @@ mod tests { assert!(update.search_values[0].enabled); } + #[test] + fn restore_applies_stored_colors() { + let mut shared = new_shared(ObserveOrigin::File( + String::from("source"), + FileFormat::Text, + PathBuf::from("source.log"), + )); + let mut registry = FilterRegistry::default(); + let mut recent = file_runtime(); + let colors = StoredColorPair { + fg: [1, 2, 3, 4], + bg: [5, 6, 7, 8], + }; + let restore_state = RecentSessionStateSnapshot { + filters: vec![RecentFilterSnapshot { + filter: SearchFilter::plain("level=warn"), + enabled: true, + colors: Some(colors), + }], + search_values: vec![RecentSearchValueSnapshot { + filter: SearchFilter::plain("cpu=(\\d+)"), + enabled: true, + color: Some([9, 10, 11, 12]), + }], + bookmarks: vec![], + }; + + recent.apply_restore(restore_state, &mut shared, &mut registry); + + assert_eq!( + shared.filters.filter_entries[0].colors, + ColorPair::new( + Color32::from_rgba_unmultiplied(1, 2, 3, 4), + Color32::from_rgba_unmultiplied(5, 6, 7, 8), + ) + ); + assert_eq!( + shared.filters.search_value_entries[0].color, + Color32::from_rgba_unmultiplied(9, 10, 11, 12) + ); + } + + #[test] + fn restore_missing_colors_uses_defaults() { + let mut shared = new_shared(ObserveOrigin::File( + String::from("source"), + FileFormat::Text, + PathBuf::from("source.log"), + )); + let mut registry = FilterRegistry::default(); + let mut recent = file_runtime(); + let restore_state = RecentSessionStateSnapshot { + filters: vec![RecentFilterSnapshot { + filter: SearchFilter::plain("level=warn"), + enabled: true, + colors: None, + }], + search_values: vec![RecentSearchValueSnapshot { + filter: SearchFilter::plain("cpu=(\\d+)"), + enabled: true, + color: None, + }], + bookmarks: vec![], + }; + + recent.apply_restore(restore_state, &mut shared, &mut registry); + + assert_eq!( + shared.filters.filter_entries[0].colors, + colors::FILTER_HIGHLIGHT_COLORS[0].clone() + ); + assert_eq!( + shared.filters.search_value_entries[0].color, + colors::search_value_color(0) + ); + } + + #[test] + fn color_only_edit_updates_recent_state() { + let mut shared = new_shared(ObserveOrigin::File( + String::from("source"), + FileFormat::Text, + PathBuf::from("source.log"), + )); + let mut registry = FilterRegistry::default(); + let filter_id = registry.add_filter(FilterDefinition::new(SearchFilter::plain("first"))); + shared.apply_filter(&mut registry, filter_id); + let mut recent = file_runtime(); + let _ = recent.capture_opened_state(&shared, ®istry); + + shared.set_filter_colors( + &filter_id, + ColorPair::new(Color32::from_rgb(1, 2, 3), Color32::from_rgb(4, 5, 6)), + ); + + let update = recent + .take_state_update(&shared, ®istry) + .expect("color-only edit should update recent state"); + + let expected_colors = StoredColorPair { + fg: [1, 2, 3, 255], + bg: [4, 5, 6, 255], + }; + assert_eq!(update.filters[0].colors, Some(expected_colors)); + } + #[test] fn restore_search_waits_for_session_file_ready() { let runtime = Runtime::new().expect("runtime should initialize"); @@ -426,13 +585,15 @@ mod tests { let filter = SearchFilter::plain("level=warn"); let search_value = SearchFilter::plain("cpu=(\\d+)"); let restore_state = RecentSessionStateSnapshot { - filters: vec![SearchFilterSnapshot { + filters: vec![RecentFilterSnapshot { filter: filter.clone(), enabled: true, + colors: None, }], - search_values: vec![SearchFilterSnapshot { + search_values: vec![RecentSearchValueSnapshot { filter: search_value.clone(), enabled: true, + color: None, }], bookmarks: vec![], }; diff --git a/crates/app/src/session/ui/shared/mod.rs b/crates/app/src/session/ui/shared/mod.rs index 5a9fefa89c..e8fa50f085 100644 --- a/crates/app/src/session/ui/shared/mod.rs +++ b/crates/app/src/session/ui/shared/mod.rs @@ -308,6 +308,28 @@ impl SessionShared { self.recent_revision } + /// Updates one applied filter color pair and tracks recent-session dirtiness. + /// + /// Returns `true` only when the stored color pair changed. + pub fn set_filter_colors(&mut self, filter_id: &Uuid, colors: ColorPair) -> bool { + let changed = self.filters.set_filter_colors(filter_id, colors); + if changed { + self.bump_recent_revision(); + } + changed + } + + /// Updates one applied search-value color and tracks recent-session dirtiness. + /// + /// Returns `true` only when the stored color changed. + pub fn set_search_value_color(&mut self, value_id: &Uuid, color: Color32) -> bool { + let changed = self.filters.set_search_value_color(value_id, color); + if changed { + self.bump_recent_revision(); + } + changed + } + /// Updates one applied filter enabled flag and tracks recent-session dirtiness. pub fn set_filter_enabled(&mut self, filter_id: &Uuid, enabled: bool) -> bool { let changed = self.filters.set_filter_enabled(filter_id, enabled); @@ -353,33 +375,21 @@ impl SessionShared { changed } - /// Applies a filter with an explicit enabled state and tracks recent-session dirtiness. + /// Applies a filter with explicit state and tracks recent-session dirtiness. + /// + /// `Some(colors)` applies explicit colors. `None` assigns default colors for + /// new rows and preserves existing row colors. Returns `true` when the row + /// was added or its stored state changed. pub fn apply_filter_with_state( &mut self, registry: &mut FilterRegistry, filter_id: Uuid, enabled: bool, + colors: Option, ) -> bool { let changed = self .filters - .apply_filter_with_state(registry, filter_id, enabled); - if changed { - self.bump_recent_revision(); - } - changed - } - - /// Applies full filter row state and tracks recent-session dirtiness. - pub fn set_filter_entry_state( - &mut self, - registry: &mut FilterRegistry, - filter_id: Uuid, - enabled: bool, - colors: ColorPair, - ) -> bool { - let changed = self - .filters - .set_filter_entry_state(registry, filter_id, enabled, colors); + .apply_filter_with_state(registry, filter_id, enabled, colors); if changed { self.bump_recent_revision(); } @@ -413,33 +423,21 @@ impl SessionShared { changed } - /// Applies a search value with an explicit enabled state and tracks recent-session dirtiness. + /// Applies a search value with explicit state and tracks recent-session dirtiness. + /// + /// `Some(color)` applies an explicit color. `None` assigns a default color + /// for new rows and preserves existing row colors. Returns `true` when the + /// row was added or its stored state changed. pub fn apply_search_value_with_state( &mut self, registry: &mut FilterRegistry, value_id: Uuid, enabled: bool, + color: Option, ) -> bool { let changed = self .filters - .apply_search_value_with_state(registry, value_id, enabled); - if changed { - self.bump_recent_revision(); - } - changed - } - - /// Applies full search-value row state and tracks recent-session dirtiness. - pub fn set_search_value_entry_state( - &mut self, - registry: &mut FilterRegistry, - value_id: Uuid, - enabled: bool, - color: Color32, - ) -> bool { - let changed = self - .filters - .set_search_value_entry_state(registry, value_id, enabled, color); + .apply_search_value_with_state(registry, value_id, enabled, color); if changed { self.bump_recent_revision(); } @@ -567,6 +565,35 @@ mod tests { shared.filters.apply_search_value(registry, value_id); } + #[test] + fn color_setters_bump_recent_revision_only_on_change() { + let mut shared = new_shared(); + let mut registry = FilterRegistry::default(); + let filter_id = + add_filter_def(&mut shared, &mut registry, SearchFilter::plain("status=ok")); + let value_def = SearchValueDefinition::new( + SearchFilter::plain("cpu=(\\d+)") + .regex(true) + .ignore_case(true), + ); + let value_id = value_def.id; + registry.add_search_value(value_def); + shared.filters.apply_search_value(&mut registry, value_id); + let baseline = shared.recent_revision(); + + let colors = ColorPair::new(Color32::from_rgb(1, 2, 3), Color32::from_rgb(4, 5, 6)); + assert!(shared.set_filter_colors(&filter_id, colors.clone())); + assert_eq!(shared.recent_revision(), baseline + 1); + assert!(!shared.set_filter_colors(&filter_id, colors)); + assert_eq!(shared.recent_revision(), baseline + 1); + + let color = Color32::from_rgb(7, 8, 9); + assert!(shared.set_search_value_color(&value_id, color)); + assert_eq!(shared.recent_revision(), baseline + 2); + assert!(!shared.set_search_value_color(&value_id, color)); + assert_eq!(shared.recent_revision(), baseline + 2); + } + #[test] fn drop_search_updates_counts() { let mut shared = new_shared(); diff --git a/crates/app/src/session/ui/shared/searching/filters.rs b/crates/app/src/session/ui/shared/searching/filters.rs index 0e6a4a02b0..e0daeb8af0 100644 --- a/crates/app/src/session/ui/shared/searching/filters.rs +++ b/crates/app/src/session/ui/shared/searching/filters.rs @@ -219,134 +219,112 @@ impl FiltersState { colors::next_search_value_color(&used_colors) } - /// Adds the filter to the session or updates its enabled state in place. - fn set_filter_entry(&mut self, registry: &mut FilterRegistry, id: Uuid, enabled: bool) -> bool { - if let Some(item) = self.filter_entries.iter_mut().find(|item| item.id == id) { - let changed = item.enabled != enabled; - item.enabled = enabled; - return changed; - } - - let colors = self.next_filter_color_pair(); - self.filter_entries - .push(AppliedFilterState::new(id, enabled, colors)); - registry.apply_filter_to_session(id, self.session_id); - true - } - - /// Adds the search value to the session or updates its enabled state in place. - fn set_search_value_entry( - &mut self, - registry: &mut FilterRegistry, - id: Uuid, - enabled: bool, - ) -> bool { - if let Some(item) = self - .search_value_entries - .iter_mut() - .find(|item| item.id == id) - { - let changed = item.enabled != enabled; - item.enabled = enabled; - return changed; - } - - let color = self.next_search_value_color(); - self.search_value_entries - .push(AppliedSearchValueState::new(id, enabled, color)); - registry.apply_search_value_to_session(id, self.session_id); - true - } - - /// Adds or updates a filter row with explicit enabled state and colors. - pub fn set_filter_entry_state( - &mut self, - registry: &mut FilterRegistry, - id: Uuid, - enabled: bool, - colors: ColorPair, - ) -> bool { - if let Some(item) = self.filter_entries.iter_mut().find(|item| item.id == id) { - let changed = item.enabled != enabled || item.colors != colors; - item.enabled = enabled; - item.colors = colors; - return changed; - } + /// Updates filter colors for an existing row. + /// + /// Returns `true` only when the row exists and its stored color pair changed. + pub fn set_filter_colors(&mut self, id: &Uuid, colors: ColorPair) -> bool { + let Some(item) = self.filter_entries.iter_mut().find(|item| item.id == *id) else { + return false; + }; - self.filter_entries - .push(AppliedFilterState::new(id, enabled, colors)); - registry.apply_filter_to_session(id, self.session_id); + let changed = item.colors != colors; + item.colors = colors; - true + changed } - /// Adds or updates a search-value row with explicit enabled state and color. - pub fn set_search_value_entry_state( - &mut self, - registry: &mut FilterRegistry, - id: Uuid, - enabled: bool, - color: Color32, - ) -> bool { - if let Some(item) = self + /// Updates search-value color for an existing row. + /// + /// Returns `true` only when the row exists and its stored color changed. + pub fn set_search_value_color(&mut self, id: &Uuid, color: Color32) -> bool { + let Some(item) = self .search_value_entries .iter_mut() - .find(|item| item.id == id) - { - let changed = item.enabled != enabled || item.color != color; - item.enabled = enabled; - item.color = color; - return changed; - } + .find(|item| item.id == *id) + else { + return false; + }; - self.search_value_entries - .push(AppliedSearchValueState::new(id, enabled, color)); - registry.apply_search_value_to_session(id, self.session_id); + let changed = item.color != color; + item.color = color; - true + changed } /// Updates the enabled flag for an existing filter and reports /// whether it changed. pub fn set_filter_enabled(&mut self, id: &Uuid, enabled: bool) -> bool { - if let Some(item) = self.filter_entries.iter_mut().find(|item| item.id == *id) { - let changed = item.enabled != enabled; - item.enabled = enabled; - return changed; - } + let Some(item) = self.filter_entries.iter_mut().find(|item| item.id == *id) else { + return false; + }; - false + let changed = item.enabled != enabled; + item.enabled = enabled; + + changed } /// Updates the enabled flag for an existing search value and reports /// whether it changed. pub fn set_search_value_enabled(&mut self, id: &Uuid, enabled: bool) -> bool { - if let Some(item) = self + let Some(item) = self .search_value_entries .iter_mut() .find(|item| item.id == *id) - { - let changed = item.enabled != enabled; - item.enabled = enabled; - return changed; - } + else { + return false; + }; - false + let changed = item.enabled != enabled; + item.enabled = enabled; + + changed } /// Adds a filter to the session in the enabled state. pub fn apply_filter(&mut self, registry: &mut FilterRegistry, id: Uuid) -> bool { - self.set_filter_entry(registry, id, true) + self.apply_filter_with_state(registry, id, true, None) } - /// Adds a filter to the session while preserving an explicit enabled state. + /// Adds or updates a filter row with explicit state. + /// + /// # Arguments + /// + /// - `registry`: Registry that tracks session usage for the filter definition. + /// - `id`: Registry ID of the filter definition to apply. + /// - `enabled`: Enabled state to store on the applied row. + /// - `colors`: Explicit row colors. `None` assigns default colors for new + /// rows and preserves existing row colors. + /// + /// # Returns + /// + /// `true` when the row was added or its stored state changed. pub fn apply_filter_with_state( &mut self, registry: &mut FilterRegistry, id: Uuid, enabled: bool, + colors: Option, ) -> bool { - self.set_filter_entry(registry, id, enabled) + if let Some(item) = self.filter_entries.iter_mut().find(|item| item.id == id) { + let changed = match &colors { + Some(colors) => item.enabled != enabled || item.colors != *colors, + None => item.enabled != enabled, + }; + item.enabled = enabled; + if let Some(colors) = colors { + item.colors = colors; + } + + return changed; + } + + let colors = colors.unwrap_or_else(|| self.next_filter_color_pair()); + self.filter_entries + .push(AppliedFilterState::new(id, enabled, colors)); + registry.apply_filter_to_session(id, self.session_id); + + true } /// Removes the filter from the session entirely. @@ -386,17 +364,51 @@ impl FiltersState { /// Adds a search value to the session in the enabled state. pub fn apply_search_value(&mut self, registry: &mut FilterRegistry, id: Uuid) -> bool { - self.set_search_value_entry(registry, id, true) + self.apply_search_value_with_state(registry, id, true, None) } - /// Adds a search value to the session while preserving an explicit enabled state. + /// Adds or updates a search-value row with explicit state. + /// + /// # Arguments + /// + /// - `registry`: Registry that tracks session usage for the search-value definition. + /// - `id`: Registry ID of the search-value definition to apply. + /// - `enabled`: Enabled state to store on the applied row. + /// - `color`: Explicit row color. `None` assigns a default color for new + /// rows and preserves existing row colors. + /// + /// # Returns + /// + /// `true` when the row was added or its stored state changed. pub fn apply_search_value_with_state( &mut self, registry: &mut FilterRegistry, id: Uuid, enabled: bool, + color: Option, ) -> bool { - self.set_search_value_entry(registry, id, enabled) + if let Some(item) = self + .search_value_entries + .iter_mut() + .find(|item| item.id == id) + { + let changed = match color { + Some(color) => { + let changed = item.enabled != enabled || item.color != color; + item.color = color; + changed + } + None => item.enabled != enabled, + }; + item.enabled = enabled; + return changed; + } + + let color = color.unwrap_or_else(|| self.next_search_value_color()); + self.search_value_entries + .push(AppliedSearchValueState::new(id, enabled, color)); + registry.apply_search_value_to_session(id, self.session_id); + true } /// Removes the search value from the session entirely. @@ -568,7 +580,7 @@ mod tests { let mut registry = FilterRegistry::default(); let filter_id = add_plain_filter_definition(&mut registry, "first"); - state.apply_filter_with_state(&mut registry, filter_id, false); + state.apply_filter_with_state(&mut registry, filter_id, false, None); let entry = state .filter_entries .iter_mut() @@ -587,6 +599,39 @@ mod tests { assert_eq!(entry.colors, ColorPair::new(Color32::WHITE, Color32::BLACK)); } + #[test] + fn set_filter_colors_reports_only_changes() { + let mut state = new_state(); + let mut registry = FilterRegistry::default(); + let filter_id = add_plain_filter_definition(&mut registry, "first"); + state.apply_filter(&mut registry, filter_id); + + let colors = ColorPair::new(Color32::from_rgb(1, 2, 3), Color32::from_rgb(4, 5, 6)); + + assert!(state.set_filter_colors(&filter_id, colors.clone())); + assert!(!state.set_filter_colors(&filter_id, colors.clone())); + assert_eq!(state.filter_entries[0].colors, colors); + assert!(!state.set_filter_colors( + &Uuid::new_v4(), + ColorPair::new(Color32::WHITE, Color32::BLACK) + )); + } + + #[test] + fn set_search_value_color_reports_only_changes() { + let mut state = new_state(); + let mut registry = FilterRegistry::default(); + let value_id = add_search_value_definition(&mut registry, "first=(\\d+)"); + state.apply_search_value(&mut registry, value_id); + + let color = Color32::from_rgb(7, 8, 9); + + assert!(state.set_search_value_color(&value_id, color)); + assert!(!state.set_search_value_color(&value_id, color)); + assert_eq!(state.search_value_entries[0].color, color); + assert!(!state.set_search_value_color(&Uuid::new_v4(), Color32::WHITE)); + } + #[test] fn toggling_filter_preserves_color() { let mut state = new_state(); @@ -725,7 +770,7 @@ mod tests { let filter_id = add_regex_filter_definition(&mut registry, "cpu=(\\d+)"); let existing_value_id = add_search_value_definition(&mut registry, "temp=(\\d+)"); - state.apply_filter_with_state(&mut registry, filter_id, false); + state.apply_filter_with_state(&mut registry, filter_id, false, None); state.apply_search_value(&mut registry, existing_value_id); let was_enabled = state.is_filter_enabled(&filter_id); @@ -734,7 +779,7 @@ mod tests { .expect("eligible filter should convert"); state.unapply_filter(&mut registry, &filter_id); - state.apply_search_value_with_state(&mut registry, converted_value_id, was_enabled); + state.apply_search_value_with_state(&mut registry, converted_value_id, was_enabled, None); let entry = state .search_value_entries @@ -752,7 +797,7 @@ mod tests { let filter_id = add_regex_filter_definition(&mut registry, "cpu=(\\d+)"); let existing_value_id = add_search_value_definition(&mut registry, "cpu=(\\d+)"); - state.apply_filter_with_state(&mut registry, filter_id, false); + state.apply_filter_with_state(&mut registry, filter_id, false, None); state.apply_search_value(&mut registry, existing_value_id); state.set_search_value_enabled(&existing_value_id, false); let original_color = state @@ -768,7 +813,7 @@ mod tests { .expect("eligible filter should convert"); state.unapply_filter(&mut registry, &filter_id); - state.apply_search_value_with_state(&mut registry, converted_value_id, was_enabled); + state.apply_search_value_with_state(&mut registry, converted_value_id, was_enabled, None); assert_eq!(converted_value_id, existing_value_id); assert_eq!(registry.search_value_map().len(), 1); @@ -785,7 +830,7 @@ mod tests { let value_id = add_search_value_definition(&mut registry, "cpu=(\\d+)"); let existing_filter_id = add_plain_filter_definition(&mut registry, "status=ok"); - state.apply_search_value_with_state(&mut registry, value_id, false); + state.apply_search_value_with_state(&mut registry, value_id, false, None); state.apply_filter(&mut registry, existing_filter_id); let was_enabled = state.is_search_value_enabled(&value_id); @@ -794,7 +839,7 @@ mod tests { .expect("search value should convert"); state.unapply_search_value(&mut registry, &value_id); - state.apply_filter_with_state(&mut registry, converted_filter_id, was_enabled); + state.apply_filter_with_state(&mut registry, converted_filter_id, was_enabled, None); let entry = state .filter_entries @@ -812,7 +857,7 @@ mod tests { let value_id = add_search_value_definition(&mut registry, "cpu=(\\d+)"); let existing_filter_id = add_regex_filter_definition(&mut registry, "cpu=(\\d+)"); - state.apply_search_value_with_state(&mut registry, value_id, false); + state.apply_search_value_with_state(&mut registry, value_id, false, None); state.apply_filter(&mut registry, existing_filter_id); state.set_filter_enabled(&existing_filter_id, false); let original_colors = state @@ -829,7 +874,7 @@ mod tests { .expect("search value should convert"); state.unapply_search_value(&mut registry, &value_id); - state.apply_filter_with_state(&mut registry, converted_filter_id, was_enabled); + state.apply_filter_with_state(&mut registry, converted_filter_id, was_enabled, None); assert_eq!(converted_filter_id, existing_filter_id); assert_eq!(registry.filters_map().len(), 1); @@ -848,7 +893,7 @@ mod tests { let replacement_id = add_plain_filter_definition(&mut registry, "replacement"); state.apply_filter(&mut registry, first_id); - state.apply_filter_with_state(&mut registry, second_id, false); + state.apply_filter_with_state(&mut registry, second_id, false, None); let original_colors = state .filter_entries .iter() @@ -874,7 +919,7 @@ mod tests { let replacement_id = add_search_value_definition(&mut registry, "replacement=(\\d+)"); state.apply_search_value(&mut registry, first_id); - state.apply_search_value_with_state(&mut registry, second_id, false); + state.apply_search_value_with_state(&mut registry, second_id, false, None); let original_color = state .search_value_entries .iter() diff --git a/crates/app/src/session/ui/side_panel/filters.rs b/crates/app/src/session/ui/side_panel/filters.rs index 4d5e9ec0e7..a3620c6aac 100644 --- a/crates/app/src/session/ui/side_panel/filters.rs +++ b/crates/app/src/session/ui/side_panel/filters.rs @@ -787,10 +787,14 @@ impl FiltersUi { }); } - fn render_color_picker_row(ui: &mut Ui, label: &str, color: &mut egui::Color32) { + /// Renders one color picker row. + /// + /// Returns `true` when egui reports that the picker changed `color`. + fn render_color_picker_row(ui: &mut Ui, label: &str, color: &mut egui::Color32) -> bool { ui.label(label); - ui.color_edit_button_srgba(color); + let changed = ui.color_edit_button_srgba(color).changed(); ui.end_row(); + changed } fn render_color_swatch(ui: &mut Ui, color: egui::Color32) { @@ -816,33 +820,43 @@ impl FiltersUi { // when the semantic definition disappears from the registry. match selected_item { SelectedSidebarItem::Filter(filter_id) => { - if shared.filters.is_filter_applied(&filter_id) - && registry.get_filter(&filter_id).is_some() - && let Some(filter_entry) = shared - .filters - .filter_entries - .iter_mut() - .find(|item| item.id == filter_id) + let filter_colors = shared + .filters + .filter_entries + .iter() + .find(|item| item.id == filter_id) + .map(|item| item.colors.clone()); + if registry.get_filter(&filter_id).is_some() + && let Some(mut filter_colors) = filter_colors { + let mut changed = false; show_side_panel_group(ui, |ui| { - Self::render_filter_editor(ui, &mut filter_entry.colors); + changed = Self::render_filter_editor(ui, &mut filter_colors); }); + if changed { + shared.set_filter_colors(&filter_id, filter_colors); + } } else { self.selected_item = None; } } SelectedSidebarItem::SearchValue(value_id) => { - if shared.filters.is_search_value_applied(&value_id) - && registry.get_search_value(&value_id).is_some() - && let Some(value_entry) = shared - .filters - .search_value_entries - .iter_mut() - .find(|item| item.id == value_id) + let search_value_color = shared + .filters + .search_value_entries + .iter() + .find(|item| item.id == value_id) + .map(|item| item.color); + if registry.get_search_value(&value_id).is_some() + && let Some(mut search_value_color) = search_value_color { + let mut changed = false; show_side_panel_group(ui, |ui| { - Self::render_search_value_editor(ui, &mut value_entry.color); + changed = Self::render_search_value_editor(ui, &mut search_value_color); }); + if changed { + shared.set_search_value_color(&value_id, search_value_color); + } } else { self.selected_item = None; } @@ -850,23 +864,33 @@ impl FiltersUi { } } - fn render_filter_editor(ui: &mut Ui, colors: &mut ColorPair) { + /// Renders the selected filter color editor. + /// + /// Returns `true` when either color picker changed `colors`. + fn render_filter_editor(ui: &mut Ui, colors: &mut ColorPair) -> bool { ui.heading(RichText::new("Filter Details").size(16.0)); ui.add_space(10.0); + let mut changed = false; Grid::new("filter_editor_colors").show(ui, |ui| { - Self::render_color_picker_row(ui, "Foreground", &mut colors.fg); - Self::render_color_picker_row(ui, "Background", &mut colors.bg); + changed |= Self::render_color_picker_row(ui, "Foreground", &mut colors.fg); + changed |= Self::render_color_picker_row(ui, "Background", &mut colors.bg); }); + changed } - fn render_search_value_editor(ui: &mut Ui, color: &mut egui::Color32) { + /// Renders the selected chart/search-value color editor. + /// + /// Returns `true` when the color picker changed `color`. + fn render_search_value_editor(ui: &mut Ui, color: &mut egui::Color32) -> bool { ui.heading(RichText::new("Chart Details").size(16.0)); ui.add_space(10.0); + let mut changed = false; Grid::new("search_value_editor_colors").show(ui, |ui| { - Self::render_color_picker_row(ui, "Color", color); + changed |= Self::render_color_picker_row(ui, "Color", color); }); + changed } /// Applies the queued sidebar mutation and dispatches any required pipeline sync commands. @@ -1062,7 +1086,7 @@ impl FiltersUi { && was_applied { shared.unapply_filter(registry, &filter_id); - shared.apply_search_value_with_state(registry, value_id, was_enabled); + shared.apply_search_value_with_state(registry, value_id, was_enabled, None); self.replace_selection( SelectedSidebarItem::Filter(filter_id), SelectedSidebarItem::SearchValue(value_id), @@ -1100,7 +1124,7 @@ impl FiltersUi { && was_applied { shared.unapply_search_value(registry, &value_id); - shared.apply_filter_with_state(registry, filter_id, was_enabled); + shared.apply_filter_with_state(registry, filter_id, was_enabled, None); self.replace_selection( SelectedSidebarItem::SearchValue(value_id), SelectedSidebarItem::Filter(filter_id), From 911a4f036794f54b4a329c824b2c24531c22cc2c Mon Sep 17 00:00:00 2001 From: Ammar Abou Zor Date: Tue, 9 Jun 2026 08:47:12 +0200 Subject: [PATCH 2/2] Recent legacy: Include filters charts and bookmarks Apply best effort to include filters, charts and bookmarks when converting recent sessions in legacy Chipmunk 3 into Chipmunk 4. This convert is still best effort skipping places where calculating checksums is needed because it'll add to much complexity for little gain --- .../service/storage/recent/legacy/actions.rs | 35 +- .../service/storage/recent/legacy/history.rs | 590 ++++++++++++++++-- 2 files changed, 575 insertions(+), 50 deletions(-) diff --git a/crates/app/src/host/service/storage/recent/legacy/actions.rs b/crates/app/src/host/service/storage/recent/legacy/actions.rs index 311af4ecef..bbbaff9b5b 100644 --- a/crates/app/src/host/service/storage/recent/legacy/actions.rs +++ b/crates/app/src/host/service/storage/recent/legacy/actions.rs @@ -1,9 +1,12 @@ //! Legacy recent-action source and parser conversion. +//! +//! Recent actions are converted into native recent-session snapshots first. Saved +//! filters/charts are attached later by matching these snapshots against legacy history +//! definitions. Keep the source identities conservative: file paths are normalized for +//! legacy case-insensitive matching, process cwd only tolerates trailing separators, and +//! no basename or extension-only fallback is used. -use std::{ - ops::Not, - path::{Path, PathBuf}, -}; +use std::path::{Path, PathBuf}; use serde_json::{Map, Value, from_value}; @@ -318,7 +321,7 @@ fn parser_name_from_name(name: &str) -> Option { match name { "Text" => Some(ParserNames::Text), "Dlt" => Some(ParserNames::Dlt), - "SomeIp" => Some(ParserNames::SomeIP), + "SomeIp" | "SomeIP" => Some(ParserNames::SomeIP), "Plugin" => Some(ParserNames::Plugins), _ => None, } @@ -468,14 +471,26 @@ pub fn path_from_object(object: &Map) -> Option { /// Normalizes paths only for case-insensitive legacy matching. pub fn normalize_path(path: &Path) -> String { + // Chipmunk 3 file descriptors stored lower-cased paths, so matching follows that. path.to_string_lossy().to_lowercase() } -fn path_to_match_string(path: &Path) -> Option { - path.as_os_str() - .is_empty() - .not() - .then(|| path.to_string_lossy().into_owned()) +/// Converts a process cwd path into the source matching identity. +pub fn path_to_match_string(path: &Path) -> Option { + process_cwd_to_match_string(&path.to_string_lossy()) +} + +/// Converts a legacy process cwd string into the source matching identity. +pub fn process_cwd_to_match_string(path: &str) -> Option { + // Chipmunk 3 can persist the same cwd with or without a trailing separator. This keeps + // exact cwd matching while avoiding that formatting-only mismatch. + if path.is_empty() { + return None; + } + + let trimmed = path.trim_end_matches(['/', '\\']); + let normalized = if trimmed.is_empty() { path } else { trimmed }; + Some(normalized.to_owned()) } fn string_field<'a>(object: &'a Map, keys: &[&str]) -> Option<&'a str> { diff --git a/crates/app/src/host/service/storage/recent/legacy/history.rs b/crates/app/src/host/service/storage/recent/legacy/history.rs index 68cd69915c..a45947d886 100644 --- a/crates/app/src/host/service/storage/recent/legacy/history.rs +++ b/crates/app/src/host/service/storage/recent/legacy/history.rs @@ -1,6 +1,23 @@ //! Legacy history definition, collection, and state import. - -use std::{collections::HashSet, path::Path}; +//! +//! Chipmunk 3 stores history separately from recent actions. Recent actions contain the +//! source/parser to reopen, while history definitions and collections contain filters, +//! charts, bookmarks, and disabled search entries. The importer joins them by matching a +//! recent action to a history definition, then applying the newest related collection. +//! +//! The real Chipmunk 3 storage uses compact keys: `f` for file definitions, `c` for +//! concat definitions, `s` for stream definitions, `p` for parser names, and `e` for +//! collection entries. Collection entries also often store the actual filter/chart data +//! as nested JSON strings. +//! +//! This importer intentionally does not reproduce Chipmunk 3 checksum matching. It only +//! matches parser kind plus normalized paths or stream identities, without broad +//! basename/extension fallbacks. That keeps one-time startup import cheap and avoids +//! hashing large files or attaching history too broadly, but anonymized legacy file +//! definitions can miss otherwise valid filters until checksum matching is explicitly +//! added. + +use std::{borrow::Cow, collections::HashSet, ops::Not, path::Path}; use log::warn; use serde_json::{Map, Value}; @@ -22,7 +39,8 @@ use super::{ LegacyStorageEntry, actions::{ MatchSource, bool_from_value, match_source_from_sources, normalize_path, parse_parser_name, - parse_sources, parse_stream_match, path_from_object, supports_bookmarks, + parse_sources, parse_stream_match, path_from_object, process_cwd_to_match_string, + supports_bookmarks, }, history_collections_path, history_definitions_path, load_optional_entries, }; @@ -94,6 +112,7 @@ fn parse_history_definition(uuid: &str, content: &Value) -> Option Option Option { if let Some(object) = content.as_object() { + // Chipmunk 3 minified history definitions use compact source keys. + if let Some(file) = object.get("f") + && let Some(path) = parse_definition_path(file) + { + return Some(MatchSource::Files(vec![path])); + } + + if let Some(files) = object.get("c").and_then(Value::as_array) + && let Some(paths) = parse_definition_paths(files) + { + return Some(MatchSource::Files(paths)); + } + + if let Some(source) = object.get("s").and_then(parse_minified_stream_match) { + return Some(source); + } + if let Some(path) = path_from_object(object) { let path = normalize_path(&path); return Some(MatchSource::Files(vec![path])); @@ -117,20 +153,52 @@ fn parse_definition_source(content: &Value) -> Option { .get("files") .or_else(|| object.get("Concat")) .and_then(Value::as_array) + && let Some(paths) = parse_definition_paths(files) { - let paths = files - .iter() - .filter_map(parse_definition_path) - .collect::>(); - if !paths.is_empty() { - return Some(MatchSource::Files(paths)); - } + return Some(MatchSource::Files(paths)); } } parse_stream_match(content) } +/// Extracts a stream source from a compact Chipmunk 3 stream descriptor. +fn parse_minified_stream_match(value: &Value) -> Option { + // Legacy streams store a source kind plus two opaque identity strings. For current + // native stream types, `ma` maps to the identity used for matching and `mi` is only + // useful as process cwd. + let object = value.as_object()?; + let source = object.get("s").and_then(Value::as_str)?; + let major = object.get("ma").and_then(Value::as_str).unwrap_or(""); + let minor = object.get("mi").and_then(Value::as_str).unwrap_or(""); + + match source { + "Process" if !major.is_empty() => Some(MatchSource::Process { + command: major.to_owned(), + cwd: process_cwd_to_match_string(minor), + }), + "TCP" if !major.is_empty() => Some(MatchSource::Tcp { + bind_addr: major.to_owned(), + }), + "UDP" if !major.is_empty() => Some(MatchSource::Udp { + bind_addr: major.to_owned(), + }), + "Serial" if !major.is_empty() => Some(MatchSource::Serial { + path: Some(major.to_owned()), + }), + _ => None, + } +} + +/// Extracts all concat definition paths, rejecting partial or empty path lists. +fn parse_definition_paths(files: &[Value]) -> Option> { + let paths = files + .iter() + .map(parse_definition_path) + .collect::>>()?; + paths.is_empty().not().then_some(paths) +} + /// Extracts one normalized definition path, returning `None` when no path is present. fn parse_definition_path(value: &Value) -> Option { if let Some(path) = value.as_str() { @@ -233,6 +301,9 @@ fn relation_ids(content: &Value) -> HashSet { /// Imports collection entries into state and returns the number of skipped entries. fn parse_collection_entries(entries: &Value, state: &mut RecentSessionStateSnapshot) -> usize { + let decoded = decode_json_string(entries); + let entries = decoded.as_ref(); + if let Some(object) = entries.as_object() { return parse_collection_entry_object(object, state); } @@ -244,7 +315,8 @@ fn parse_collection_entries(entries: &Value, state: &mut RecentSessionStateSnaps items .iter() .map(|item| { - if let Some(object) = item.as_object() { + let decoded = decode_json_string(item); + if let Some(object) = decoded.as_ref().as_object() { parse_collection_entry_object(object, state) } else { 1 @@ -261,32 +333,124 @@ fn parse_collection_entry_object( let mut skipped = 0usize; let entry_type = object.get("type").and_then(Value::as_str); + let mut has_named_payload = false; for (key, value) in object { match key.as_str() { - "filters" => skipped += parse_filters(value, true, &mut state.filters), - "charts" => skipped += parse_charts(value, true, &mut state.search_values), - "bookmark" | "bookmarks" => skipped += parse_bookmarks(value, &mut state.bookmarks), - "disabled" => skipped += parse_disabled(value, state), - "type" => {} - _ => match entry_type { - Some("filters") => skipped += parse_filters(value, true, &mut state.filters), - Some("charts") => skipped += parse_charts(value, true, &mut state.search_values), - Some("bookmark") => skipped += parse_bookmarks(value, &mut state.bookmarks), - _ => {} - }, + "filters" => { + has_named_payload = true; + skipped += parse_filters(value, true, &mut state.filters); + } + "charts" => { + has_named_payload = true; + skipped += parse_charts(value, true, &mut state.search_values); + } + "bookmark" | "bookmarks" => { + has_named_payload = true; + skipped += parse_bookmarks(value, &mut state.bookmarks); + } + "disabled" => { + has_named_payload = true; + skipped += parse_disabled(value, state); + } + _ => {} } } + if !has_named_payload && let Some(entry_type) = entry_type { + skipped += parse_typed_entry_object(entry_type, object, state); + } + skipped } +/// Imports one typed collection entry envelope into state. +fn parse_typed_entry_object( + entry_type: &str, + object: &Map, + state: &mut RecentSessionStateSnapshot, +) -> usize { + match entry_type { + "filters" => { + if object.contains_key("filter") || object.contains_key("text") { + let entry = Value::Object(object.clone()); + return parse_filters(&entry, true, &mut state.filters); + } + typed_entry_payload(object) + .map(|value| parse_filters(value, true, &mut state.filters)) + .unwrap_or(0) + } + "charts" => { + if object.contains_key("chart") + || object.contains_key("filter") + || object.contains_key("text") + { + let entry = Value::Object(object.clone()); + return parse_charts(&entry, true, &mut state.search_values); + } + typed_entry_payload(object) + .map(|value| parse_charts(value, true, &mut state.search_values)) + .unwrap_or(0) + } + "bookmark" | "bookmarks" => { + if object.contains_key("position") { + let entry = Value::Object(object.clone()); + return parse_bookmarks(&entry, &mut state.bookmarks); + } + typed_entry_payload(object) + .map(|value| parse_bookmarks(value, &mut state.bookmarks)) + .unwrap_or(0) + } + _ => 0, + } +} + +/// Returns the payload from a typed collection entry, ignoring envelope metadata. +fn typed_entry_payload(object: &Map) -> Option<&Value> { + for key in ["value", "payload", "entry", "data"] { + if let Some(value) = object.get(key) { + return Some(value); + } + } + + let mut payload = None; + for (key, value) in object { + if is_typed_entry_metadata(key) { + continue; + } + if payload.is_some() { + return None; + } + payload = Some(value); + } + + payload +} + +fn is_typed_entry_metadata(key: &str) -> bool { + matches!( + key, + "type" + | "key" + | "uuid" + | "id" + | "name" + | "active" + | "enabled" + | "flags" + | "colors" + | "color" + | "widths" + ) +} + /// Imports filter entries into `output` and returns the number of skipped entries. fn parse_filters( value: &Value, default_enabled: bool, output: &mut Vec, ) -> usize { - parse_one_or_many(value, |item| parse_filter(item, default_enabled), output) + let mut parse = |item: &Value| parse_filter(item, default_enabled); + parse_one_or_many(value, &mut parse, output) } /// Imports chart search-value entries into `output` and returns the number of skipped entries. @@ -295,19 +459,24 @@ fn parse_charts( default_enabled: bool, output: &mut Vec, ) -> usize { - parse_one_or_many(value, |item| parse_chart(item, default_enabled), output) + let mut parse = |item: &Value| parse_chart(item, default_enabled); + parse_one_or_many(value, &mut parse, output) } /// Parses either one legacy entry or an array, returning the number of skipped items. fn parse_one_or_many( value: &Value, - mut parse: impl FnMut(&Value) -> Option, + parse: &mut impl FnMut(&Value) -> Option, output: &mut Vec, ) -> usize { + let decoded = decode_json_string(value); + let value = decoded.as_ref(); + if let Some(items) = value.as_array() { - let initial_len = output.len(); - output.extend(items.iter().filter_map(&mut parse)); - items.len() - (output.len() - initial_len) + items + .iter() + .map(|item| parse_one_or_many(item, parse, output)) + .sum() } else if let Some(item) = parse(value) { output.push(item); 0 @@ -316,8 +485,21 @@ fn parse_one_or_many( } } +/// Decodes legacy collection payloads that store the actual entry as a JSON string. +fn decode_json_string(value: &Value) -> Cow<'_, Value> { + if let Value::String(raw) = value + && let Ok(decoded) = serde_json::from_str::(raw) + { + return Cow::Owned(decoded); + } + + Cow::Borrowed(value) +} + /// Parses one filter snapshot, returning `None` when filter text is missing. fn parse_filter(value: &Value, default_enabled: bool) -> Option { + let decoded = decode_json_string(value); + let value = decoded.as_ref(); let filter_value = value.get("filter").unwrap_or(value); let text = if let Some(text) = filter_value.as_str() { text @@ -328,9 +510,10 @@ fn parse_filter(value: &Value, default_enabled: bool) -> Option Option Option bool { + bool_from_value(filter_value.get(key)) + .or_else(|| { + filter_value + .get("flags") + .and_then(|flags| bool_from_value(flags.get(key))) + }) + .or_else(|| bool_from_value(value.get(key))) + .or_else(|| { + value + .get("flags") + .and_then(|flags| bool_from_value(flags.get(key))) + }) + .unwrap_or(false) +} + /// Parses one chart search-value snapshot, returning `None` when filter text is missing. fn parse_chart(value: &Value, default_enabled: bool) -> Option { + let decoded = decode_json_string(value); + let value = decoded.as_ref(); let chart_value = value.get("chart").unwrap_or(value); let text = chart_value .get("filter") @@ -426,18 +628,18 @@ fn hex_value(byte: u8) -> Option { /// Imports bookmark positions into `output` and returns the number of skipped entries. fn parse_bookmarks(value: &Value, output: &mut Vec) -> usize { - parse_one_or_many( - value, - |item| { - item.as_u64() - .or_else(|| item.get("position").and_then(Value::as_u64)) - }, - output, - ) + let mut parse = |item: &Value| { + item.as_u64() + .or_else(|| item.get("position").and_then(Value::as_u64)) + }; + parse_one_or_many(value, &mut parse, output) } /// Imports disabled filters or charts into state and returns the number of skipped entries. fn parse_disabled(value: &Value, state: &mut RecentSessionStateSnapshot) -> usize { + let decoded = decode_json_string(value); + let value = decoded.as_ref(); + if let Some(items) = value.as_array() { return items.iter().map(|item| parse_disabled(item, state)).sum(); } @@ -447,6 +649,31 @@ fn parse_disabled(value: &Value, state: &mut RecentSessionStateSnapshot) -> usiz }; let mut skipped = 0usize; + if let (Some(key), Some(value)) = ( + object.get("key").and_then(Value::as_str), + object.get("value"), + ) { + // DisabledRequest stores which collection the nested JSON payload came from. + match key { + "filters" => { + let first_added = state.filters.len(); + skipped += parse_filters(value, false, &mut state.filters); + for filter in &mut state.filters[first_added..] { + filter.enabled = false; + } + return skipped; + } + "charts" => { + let first_added = state.search_values.len(); + skipped += parse_charts(value, false, &mut state.search_values); + for chart in &mut state.search_values[first_added..] { + chart.enabled = false; + } + return skipped; + } + _ => {} + } + } if let Some(filters) = object.get("filters").or_else(|| object.get("filter")) { let first_added = state.filters.len(); skipped += parse_filters(filters, false, &mut state.filters); @@ -535,10 +762,12 @@ impl LegacyHistory { .iter() .any(|id| definition_ids.contains(id.as_str())) }) + // Chipmunk 3 can keep several collections for the same definition set. .max_by_key(|collection| collection.last_used) .map(|collection| { let mut state = collection.state.clone(); if !supports_bookmarks(sources) { + // Legacy bookmarks are line offsets and only make sense for file sessions. state.bookmarks.clear(); } state @@ -551,7 +780,7 @@ mod tests { use std::path::PathBuf; use serde_json::json; - use stypes::{FileFormat, TCPTransportConfig, Transport}; + use stypes::{FileFormat, ProcessTransportConfig, TCPTransportConfig, Transport}; use super::*; @@ -674,6 +903,217 @@ mod tests { ); } + #[test] + fn parses_minified_file_definition() { + let content = json!({ + "f": { + "e": ".log", + "n": "App.log", + "p": "/LOGS", + "s": 141, + "c": 1_729_517_173_233.153_f64, + "h": "ignored" + }, + "p": "Text", + "u": "definition-id" + }); + let definition = + parse_history_definition("definition-id", &content).expect("definition should parse"); + + assert_eq!(definition.uuid, "definition-id"); + assert_eq!(definition.parser, ParserNames::Text); + assert_eq!( + definition.source, + MatchSource::Files(vec![String::from("/logs/app.log")]) + ); + } + + #[test] + fn parses_minified_concat_definition() { + let content = json!({ + "c": [ + { "e": ".log", "n": "First.log", "p": "/LOGS", "h": "a" }, + { "e": ".log", "n": "Second.log", "p": "/LOGS", "h": "b" } + ], + "p": "Text", + "u": "definition-id" + }); + let definition = + parse_history_definition("definition-id", &content).expect("definition should parse"); + + assert_eq!(definition.parser, ParserNames::Text); + assert_eq!( + definition.source, + MatchSource::Files(vec![ + String::from("/logs/first.log"), + String::from("/logs/second.log") + ]) + ); + } + + #[test] + fn rejects_partial_minified_concat_definition() { + let content = json!({ + "c": [ + { "e": ".log", "n": "First.log", "p": "/LOGS", "h": "a" }, + { "h": "missing-path" } + ], + "p": "Text", + "u": "definition-id" + }); + + assert!(parse_history_definition("definition-id", &content).is_none()); + } + + #[test] + fn parses_minified_stream_definition() { + let content = json!({ + "s": { "s": "TCP", "mi": "", "ma": "127.0.0.1:7777" }, + "p": "Dlt", + "u": "definition-id" + }); + let definition = + parse_history_definition("definition-id", &content).expect("definition should parse"); + + assert_eq!(definition.parser, ParserNames::Dlt); + assert_eq!( + definition.source, + MatchSource::Tcp { + bind_addr: String::from("127.0.0.1:7777") + } + ); + } + + #[test] + fn attaches_minified_file_history_to_matching_recent_source() { + let definition_content = json!({ + "f": { "e": ".dlt", "n": "images.dlt", "p": "/home/user/Desktop", "h": "ignored" }, + "p": "Dlt", + "u": "definition-id" + }); + let definition = parse_history_definition("definition-id", &definition_content) + .expect("definition should parse"); + let collection_content = json!({ + "r": ["definition-id"], + "l": 10, + "e": [{ "filters": { "filter": "log" } }] + }); + let collection = + parse_history_collection(&collection_content).expect("collection should parse"); + let history = LegacyHistory { + definitions: vec![definition], + collections: vec![collection], + }; + let sources = vec![RecentSessionSource::File { + format: FileFormat::Binary, + path: PathBuf::from("/home/user/Desktop/images.dlt"), + }]; + + let state = history + .state_for(&sources, ParserNames::Dlt) + .expect("history should match"); + + assert_eq!(state.filters.len(), 1); + assert_eq!(state.filters[0].filter.value, "log"); + } + + #[test] + fn imports_flat_filter_flags() { + let state = parsed_state(json!({ + "filters": [ + { + "filter": "nested flags", + "flags": { "cases": true, "word": true, "reg": true } + }, + { + "filter": "sibling flags", + "cases": true, + "word": true, + "reg": true + } + ] + })); + + assert_eq!(state.filters.len(), 2); + for filter in state.filters { + assert!(filter.filter.is_regex()); + assert!(filter.filter.is_word()); + assert!(!filter.filter.is_ignore_case()); + } + } + + #[test] + fn imports_typed_collection_entry_payload_once() { + let filter = json!({ + "filter": "log", + "flags": { "cases": false, "word": true, "reg": true }, + "active": true + }); + + let state = parsed_state(json!({ + "type": "filters", + "key": "filters", + "value": filter.to_string(), + "uuid": "entry-id" + })); + + assert_eq!(state.filters.len(), 1); + assert_eq!(state.filters[0].filter.value, "log"); + assert!(state.filters[0].filter.is_regex()); + assert!(state.filters[0].filter.is_word()); + } + + #[test] + fn imports_nested_json_string_collection_entries() { + let filter = json!({ + "filter": { + "filter": "log", + "flags": { "cases": false, "word": true, "reg": true } + }, + "uuid": "filter-id", + "active": true, + "colors": { "color": "#000000", "background": "#e4e15b" } + }); + let chart = json!({ + "filter": "value=(\\d+)", + "uuid": "chart-id", + "active": true, + "color": "#010203" + }); + let disabled_filter = json!({ + "key": "filters", + "value": filter.to_string() + }); + + let state = parsed_state(json!([ + { "filters": filter.to_string() }, + { "charts": chart.to_string() }, + { "bookmark": json!({ "position": 42 }).to_string() }, + { "disabled": disabled_filter.to_string() } + ])); + + assert_eq!(state.filters.len(), 2); + let enabled_filter = &state.filters[0]; + assert_eq!(enabled_filter.filter.value, "log"); + assert!(enabled_filter.filter.is_regex()); + assert!(enabled_filter.filter.is_word()); + assert!(enabled_filter.filter.is_ignore_case()); + assert!(enabled_filter.enabled); + assert_eq!( + enabled_filter.colors, + Some(StoredColorPair { + fg: [0, 0, 0, 255], + bg: [228, 225, 91, 255] + }) + ); + assert!(!state.filters[1].enabled); + + assert_eq!(state.search_values.len(), 1); + assert_eq!(state.search_values[0].filter.value, "value=(\\d+)"); + assert_eq!(state.search_values[0].color, Some([1, 2, 3, 255])); + assert_eq!(state.bookmarks, vec![42]); + } + /// Verifies that stream history returns state with bookmarks removed. #[test] fn does_not_attach_bookmarks_to_stream_history() { @@ -708,6 +1148,42 @@ mod tests { assert!(state.bookmarks.is_empty()); } + #[test] + fn matches_process_history_with_trailing_cwd_separator() { + let sources = vec![RecentSessionSource::Stream { + transport: Transport::Process(ProcessTransportConfig { + cwd: PathBuf::from("/home/user/"), + command: String::from("ls"), + shell: None, + }), + }]; + let relation_ids = HashSet::from([String::from("definition-id")]); + let state = parsed_state(json!({ "filters": { "filter": "log" } })); + let history = LegacyHistory { + definitions: vec![HistoryDefinition { + uuid: String::from("definition-id"), + source: MatchSource::Process { + command: String::from("ls"), + cwd: Some(String::from("/home/user")), + }, + parser: ParserNames::Text, + }], + collections: vec![HistoryCollection { + relation_ids, + last_used: 1, + state, + skipped_entries: 0, + }], + }; + + let state = history + .state_for(&sources, ParserNames::Text) + .expect("history should match"); + + assert_eq!(state.filters.len(), 1); + assert_eq!(state.filters[0].filter.value, "log"); + } + /// Verifies that unmatched history returns no state. #[test] fn leaves_history_empty_without_matching_definition() { @@ -735,6 +1211,40 @@ mod tests { assert!(history.state_for(&sources, ParserNames::Text).is_none()); } + #[test] + fn does_not_match_minified_file_definition_by_checksum() { + // Documents the deliberate no-checksum trade-off in the legacy importer. + let content = json!({ + "f": { + "e": ".dlt", + "n": "anonymized.dlt", + "p": "/legacy/anonymized", + "h": "matching-checksum-is-ignored" + }, + "p": "Dlt", + "u": "definition-id" + }); + let definition = + parse_history_definition("definition-id", &content).expect("definition should parse"); + let relation_ids = HashSet::from([String::from("definition-id")]); + let state = parsed_state(json!({ "filters": { "filter": "log" } })); + let history = LegacyHistory { + definitions: vec![definition], + collections: vec![HistoryCollection { + relation_ids, + last_used: 1, + state, + skipped_entries: 0, + }], + }; + let sources = vec![RecentSessionSource::File { + format: FileFormat::Binary, + path: PathBuf::from("/home/user/images.dlt"), + }]; + + assert!(history.state_for(&sources, ParserNames::Dlt).is_none()); + } + /// Parses test entries into state and asserts that no entries were skipped. fn parsed_state(entries: Value) -> RecentSessionStateSnapshot { let mut state = RecentSessionStateSnapshot::default();