Open
Conversation
Collaborator
Author
|
draft ui: 
|
kruss
force-pushed
the
add_native_home_screen
branch
7 times, most recently
from
b30c2f3 to
73c28a7
Compare
* Create the app crate to be run point for Chipmunk native desktop application. * Set unified version and rust edition in workspace and make all other crates follow it. * Add dependencies for native app. * Add support for parsing CLI arguments * Support for basic logging. * Hello world from native UI
* Specify the paths for internal crates in indexer workspace instead of having to specify their paths in each project. * Apply those changes on all internal crates withing indexer directory
* Rename crate to application * Define UI & Core & Service and State modules. * Define communication types (commands and events) for both general app and sessions. * Define communication channels for general app * Render Menu with one command to close the app.
+ Ignore errors in repaint watcher since we will get and error once the states sender channel is dropped.
* Use two main modules for the app (host and session) and have each modules containing its UI, services, data and core modules as sub-modules. * This change will make it easier in the future to split the two main modules into separated crates.
* Create the UI for rendering the content of the sessions using the communication channels. * Spawn a task for each session to make the communication for each session locally. * UI will just read the file and render it's content as lines. * Channels communication still need some thinking because we are tight to the render loop to receive the events.
Change the mechanism for waking up the UI on events to include egui context in all senders and call it explicitly on each send.
* Wire up close session command adding and ID for each session. * Improve notifications adding additional items and cleaning up the implementation in UI.
* Log send errors in one function and embedded it in all senders
* Create a struct to be passed through UI components to be able to send notifications and check for shared UI state across all UI components. * Use this struct to send command encapsulating the error handling. * New error type for UI.
* Rename to UiComponents to HostUI * Move receivers and notification handling to host UI completely.
* Use the table provided by the crate egui_table to show the logs. * Current implementation still primitive without using functionality in chipmunk core.
We need the UI for each session to have it's own unique ID to avoid its owned controls from having ID clashes. We had this problem with tables from different sessions sharing their scroll state.
* Bind session from rust core libraries in the native UI. * Currently we can establish a session and deliver the number of lines that has been read. * UI needs to request for lines so it can display them in the table.
Building system to display logs in table (WIP): We load all logs without cleaning up the logs aren't needed.
* Better way to check for data need to be fetched. * Encapsulate logic for getting log entries inside MainTable struct
* This approach avoids having to load existing logs but is complex and is based on assumptions that the logs are sequenced and sorted.
* Implement showing a window of logs simply using a map with indexes avoiding the complexity of keeping track on the existing logs. * Request discard rendering frame when new logs needed to be fetched to avoid rendering the rows where we don't have data.
kruss
force-pushed
the
add_native_home_screen
branch
13 times, most recently
from
284c188 to
6bc2891
Compare
Move color assignment from global registry to each session since it's a visual setting that shouldn't be a part of the logical parameters for each session
Previous implementation had multiple boolean in constructor leading to
code like new("search", true, false, true)
This is replaced with the builder pattern
* Filter flags can be edited after being applied UI. * Remove and move to charts are removed since they are available in the context menu anyway * Registry supported editing filter with duplicating them if a filter is referenced in multiple places. * Filter and charts text values can be edited with text edit mode. * Add regex validation for filters when regex flag is enabled. * Registry checks for duplicate when adding new filters and search values.
kruss
force-pushed
the
add_native_home_screen
branch
from
6bc2891 to
3d0f2ea
Compare
kruss
force-pushed
the
add_native_home_screen
branch
from
3d0f2ea to
bc0ea26
Compare
* Implement bookmarks log items adding them to indexed table with manual clicks. * Fixes for keeping track on search counts distinguishing it from the total indexed count.
* Support for growing data. * Support for zoom out of boundary use-case.
* Add development menu entry in debug builds only. * Add command to reset egui memory in development menu.
kruss
force-pushed
the
add_native_home_screen
branch
from
bc0ea26 to
83bb8e8
Compare
kruss
force-pushed
the
add_native_home_screen
branch
from
83bb8e8 to
300c33b
Compare
AmmarAbouZor
requested changes
Mar 17, 2026
Member
AmmarAbouZor
left a comment
AmmarAbouZor
left a comment
There was a problem hiding this comment.
I reviewed some files and left some comments.
However, we have some fundamental design points that we still need to implement here:
- We need unified
SorageServicewhich handles storing and loading, which we can extend for later features in this re-write (think about saving app configurations, presets, filters ...etc) - Recent Session should be a service so we don't scan directories and do other IO operations in the UI thread. (There are other comments in the code). Also, for that we need to think that each session should be able to reach those recent sessions to update them when filters, charts and bookmarks are applied
Other than that:
- I've found some (what I would assume) a left over and formatting errors in some files which needs to be reverted
- We have some comments about returning Result instead of Option
- The first commit in this branch is almost an empty one so I would squash it with the more last one
| use stypes::{FileFormat, ObserveOptions, ObserveOrigin, ParserType, Transport}; | ||
|
|
||
| #[derive(Serialize, Deserialize, Default, Debug)] | ||
| pub struct HomeSettings { |
Member
There was a problem hiding this comment.
I would rename the struct to HomeState and the module to state instead of settings as we are not saving state here and not configurations
| } | ||
|
|
||
| impl HomeSettings { | ||
| pub fn load(path: &std::path::Path) -> Option<Self> { |
Member
There was a problem hiding this comment.
I would make the function returns the results instead of swallowing it silently. Then we can display it in the app notifications and log it properly
Comment on lines
+28
to
+32
| pub fn save(&self, path: &std::path::Path) { | ||
| if let Ok(data) = serde_json::to_string_pretty(self) { | ||
| let _ = fs::write(path, data); | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
Also this shouldn't fail silently. I would return a result here as well and do a proper error handling
Comment on lines
+62
to
+76
| if let Some(cfg) = self.configurations.first() { | ||
| return match &cfg.options.origin { | ||
| ObserveOrigin::File(_, format, _) => { | ||
| if *format == FileFormat::Text { | ||
| None | ||
| } else { | ||
| Some(cfg) | ||
| } | ||
| } | ||
| ObserveOrigin::Concat(_) => Some(cfg), | ||
| ObserveOrigin::Stream(_, _) => None, | ||
| }; | ||
| } | ||
|
|
||
| None |
Member
There was a problem hiding this comment.
Nit: This can be written in more concise way
self.configurations
.first()
.and_then(|cfg| match &cfg.options.origin {
ObserveOrigin::File(_, format, _) if *format == FileFormat::Text => None,
ObserveOrigin::File(..) | ObserveOrigin::Concat(..) => Some(cfg),
ObserveOrigin::Stream(..) => None,
})| } | ||
|
|
||
| impl SessionConfig { | ||
| pub fn from_observe_options(options: &ObserveOptions) -> Option<Self> { |
Member
There was a problem hiding this comment.
- We need to return a result here instead of swallowing the error
- I think we can make the function gets the
ObserveOptionsby value instead of by reference since it needs the values (See the clone() call down) With that the caller may be able to pass the ownership of the item to the function so we don't need to clone it
Comment on lines
+290
to
+309
| fn file_size(bytes: u64) -> String { | ||
| const KB: f64 = 1024.0; | ||
| const MB: f64 = KB * 1024.0; | ||
| const GB: f64 = MB * 1024.0; | ||
| const TB: f64 = GB * 1024.0; | ||
|
|
||
| let bytes = bytes as f64; | ||
|
|
||
| if bytes >= TB { | ||
| format!("{:.2} TB", bytes / TB) | ||
| } else if bytes >= GB { | ||
| format!("{:.2} GB", bytes / GB) | ||
| } else if bytes >= MB { | ||
| format!("{:.2} MB", bytes / MB) | ||
| } else if bytes >= KB { | ||
| format!("{:.2} KB", bytes / KB) | ||
| } else { | ||
| format!("{} B", bytes as u64) | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
We have the same function here file_utls::format_file_size()
| @@ -1,6 +1,5 @@ | |||
| pub mod dlt; | |||
| pub mod someip; | |||
|
|
|||
Member
There was a problem hiding this comment.
formatting error, this should be reverted
| @@ -1,6 +1,5 @@ | |||
| use std::path::PathBuf; | |||
|
|
|||
| use itertools::Itertools; | |||
Member
There was a problem hiding this comment.
formatting error, this should be reverted
| @@ -1,5 +1,4 @@ | |||
| use std::path::PathBuf; | |||
|
|
|||
Member
There was a problem hiding this comment.
formatting error, this should be reverted
| pub mod udp; | ||
|
|
||
| use std::path::PathBuf; | ||
|
|
Member
There was a problem hiding this comment.
formatting error, this should be reverted
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a native-ui home screen with support for quick-actions, recent configurations and favorite folders.