feat: add desktop startup tray settings#138
Merged
zouyonghe merged 2 commits intoJun 12, 2026
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces desktop settings to the Tauri application, including 'Launch at Login', 'Silent Launch', and 'Close to Tray' options, and integrates them into the tray menu and window event handling. Feedback on these changes highlights two key improvements: first, deferring the disk read of desktop settings in app_runtime.rs to only the specific window events that require them to avoid performance degradation; second, utilizing tempfile::tempdir() in the desktop_settings.rs tests to ensure temporary directories are automatically cleaned up.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
29
to
50
| let desktop_settings = desktop_settings::read_desktop_settings( | ||
| runtime_paths::default_packaged_root_dir().as_deref(), | ||
| ); | ||
| let action = match &event { | ||
| WindowEvent::CloseRequested { .. } => app_runtime_events::main_window_action( | ||
| window.label(), | ||
| is_quitting, | ||
| false, | ||
| true, | ||
| false, | ||
| desktop_settings.close_to_tray, | ||
| ), | ||
| WindowEvent::Focused(false) => app_runtime_events::main_window_action( | ||
| window.label(), | ||
| is_quitting, | ||
| matches!(window.is_minimized(), Ok(true)), | ||
| false, | ||
| true, | ||
| desktop_settings.close_to_tray, | ||
| ), | ||
| _ => app_runtime_events::MainWindowAction::None, | ||
| }; |
Contributor
There was a problem hiding this comment.
Reading the desktop settings from disk on every single window event is highly inefficient. Window events fire extremely frequently (e.g., during window resizing, dragging, or mouse movement). Defer reading the settings from disk until we actually match the specific events that require them (CloseRequested and Focused(false)).
let action = match &event {
WindowEvent::CloseRequested { .. } => {
let desktop_settings = desktop_settings::read_desktop_settings(
runtime_paths::default_packaged_root_dir().as_deref(),
);
app_runtime_events::main_window_action(
window.label(),
is_quitting,
false,
true,
false,
desktop_settings.close_to_tray,
)
}
WindowEvent::Focused(false) => {
let desktop_settings = desktop_settings::read_desktop_settings(
runtime_paths::default_packaged_root_dir().as_deref(),
);
app_runtime_events::main_window_action(
window.label(),
is_quitting,
matches!(window.is_minimized(), Ok(true)),
false,
true,
desktop_settings.close_to_tray,
)
}
_ => app_runtime_events::MainWindowAction::None,
};
Comment on lines
+171
to
+281
| #[cfg(test)] | ||
| mod tests { | ||
| use std::{fs, path::PathBuf}; | ||
|
|
||
| use super::*; | ||
|
|
||
| fn create_temp_case_dir(name: &str) -> PathBuf { | ||
| let ts = std::time::SystemTime::now() | ||
| .duration_since(std::time::UNIX_EPOCH) | ||
| .expect("system time before unix epoch") | ||
| .as_nanos(); | ||
| std::env::temp_dir().join(format!( | ||
| "astrbot-desktop-settings-test-{}-{}-{}", | ||
| std::process::id(), | ||
| name, | ||
| ts | ||
| )) | ||
| } | ||
|
|
||
| fn state_path(root: &std::path::Path) -> PathBuf { | ||
| root.join("data").join("desktop_state.json") | ||
| } | ||
|
|
||
| #[test] | ||
| fn desktop_settings_default_preserves_existing_close_to_tray_behavior() { | ||
| assert_eq!( | ||
| DesktopSettings::default(), | ||
| DesktopSettings { | ||
| launch_at_login: false, | ||
| silent_launch: false, | ||
| close_to_tray: true, | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn read_desktop_settings_maps_camel_case_fields() { | ||
| let root = create_temp_case_dir("read"); | ||
| let path = state_path(&root); | ||
| fs::create_dir_all(path.parent().expect("state parent")).expect("create state parent"); | ||
| fs::write( | ||
| &path, | ||
| r#"{"launchAtLogin":true,"silentLaunch":true,"closeToTray":false}"#, | ||
| ) | ||
| .expect("write state"); | ||
|
|
||
| assert_eq!( | ||
| read_desktop_settings(Some(&root)), | ||
| DesktopSettings { | ||
| launch_at_login: true, | ||
| silent_launch: true, | ||
| close_to_tray: false, | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn write_desktop_setting_preserves_unknown_fields() { | ||
| let root = create_temp_case_dir("preserve"); | ||
| let path = state_path(&root); | ||
| fs::create_dir_all(path.parent().expect("state parent")).expect("create state parent"); | ||
| fs::write( | ||
| &path, | ||
| r#"{"locale":"zh-CN","updateChannel":"nightly","silentLaunch":false}"#, | ||
| ) | ||
| .expect("write state"); | ||
|
|
||
| let updated = write_desktop_setting(Some(&root), DesktopSettingKey::SilentLaunch, true) | ||
| .expect("write setting"); | ||
|
|
||
| assert!(updated.silent_launch); | ||
| let raw = fs::read_to_string(&path).expect("read state"); | ||
| let parsed: serde_json::Value = serde_json::from_str(&raw).expect("parse state"); | ||
| assert_eq!( | ||
| parsed.get("locale").and_then(|value| value.as_str()), | ||
| Some("zh-CN") | ||
| ); | ||
| assert_eq!( | ||
| parsed.get("updateChannel").and_then(|value| value.as_str()), | ||
| Some("nightly") | ||
| ); | ||
| assert_eq!( | ||
| parsed.get("silentLaunch").and_then(|value| value.as_bool()), | ||
| Some(true) | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn invalid_state_falls_back_to_defaults_and_write_resets_object() { | ||
| let root = create_temp_case_dir("invalid"); | ||
| let path = state_path(&root); | ||
| fs::create_dir_all(path.parent().expect("state parent")).expect("create state parent"); | ||
| fs::write(&path, "not-json").expect("write invalid state"); | ||
|
|
||
| assert_eq!( | ||
| read_desktop_settings(Some(&root)), | ||
| DesktopSettings::default() | ||
| ); | ||
|
|
||
| let updated = write_desktop_setting(Some(&root), DesktopSettingKey::CloseToTray, false) | ||
| .expect("write setting"); | ||
|
|
||
| assert!(!updated.close_to_tray); | ||
| let raw = fs::read_to_string(&path).expect("read state"); | ||
| let parsed: serde_json::Value = serde_json::from_str(&raw).expect("parse reset state"); | ||
| assert_eq!( | ||
| parsed.get("closeToTray").and_then(|value| value.as_bool()), | ||
| Some(false) | ||
| ); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Manually constructing temporary directories under std::env::temp_dir() leaves leftover directories on the disk after tests finish, which is poor test hygiene. Since tempfile is already a dev-dependency in Cargo.toml, use tempfile::tempdir() to automatically clean up the temporary directories when the test finishes.
#[cfg(test)]
mod tests {
use std::fs;
use super::*;
fn state_path(root: &std::path::Path) -> std::path::PathBuf {
root.join("data").join("desktop_state.json")
}
#[test]
fn desktop_settings_default_preserves_existing_close_to_tray_behavior() {
assert_eq!(
DesktopSettings::default(),
DesktopSettings {
launch_at_login: false,
silent_launch: false,
close_to_tray: true,
}
);
}
#[test]
fn read_desktop_settings_maps_camel_case_fields() {
let tmp_dir = tempfile::tempdir().expect("create temp dir");
let root = tmp_dir.path();
let path = state_path(root);
fs::create_dir_all(path.parent().expect("state parent")).expect("create state parent");
fs::write(
&path,
r#"{"launchAtLogin":true,"silentLaunch":true,"closeToTray":false}"#,
)
.expect("write state");
assert_eq!(
read_desktop_settings(Some(root)),
DesktopSettings {
launch_at_login: true,
silent_launch: true,
close_to_tray: false,
}
);
}
#[test]
fn write_desktop_setting_preserves_unknown_fields() {
let tmp_dir = tempfile::tempdir().expect("create temp dir");
let root = tmp_dir.path();
let path = state_path(root);
fs::create_dir_all(path.parent().expect("state parent")).expect("create state parent");
fs::write(
&path,
r#"{"locale":"zh-CN","updateChannel":"nightly","silentLaunch":false}"#,
)
.expect("write state");
let updated = write_desktop_setting(Some(root), DesktopSettingKey::SilentLaunch, true)
.expect("write setting");
assert!(updated.silent_launch);
let raw = fs::read_to_string(&path).expect("read state");
let parsed: serde_json::Value = serde_json::from_str(&raw).expect("parse state");
assert_eq!(
parsed.get("locale").and_then(|value| value.as_str()),
Some("zh-CN")
);
assert_eq!(
parsed.get("updateChannel").and_then(|value| value.as_str()),
Some("nightly")
);
assert_eq!(
parsed.get("silentLaunch").and_then(|value| value.as_bool()),
Some(true)
);
}
#[test]
fn invalid_state_falls_back_to_defaults_and_write_resets_object() {
let tmp_dir = tempfile::tempdir().expect("create temp dir");
let root = tmp_dir.path();
let path = state_path(root);
fs::create_dir_all(path.parent().expect("state parent")).expect("create state parent");
fs::write(&path, "not-json").expect("write invalid state");
assert_eq!(
read_desktop_settings(Some(root)),
DesktopSettings::default()
);
let updated = write_desktop_setting(Some(root), DesktopSettingKey::CloseToTray, false)
.expect("write setting");
assert!(!updated.close_to_tray);
let raw = fs::read_to_string(&path).expect("read state");
let parsed: serde_json::Value = serde_json::from_str(&raw).expect("parse reset state");
assert_eq!(
parsed.get("closeToTray").and_then(|value| value.as_bool()),
Some(false)
);
}
}
zouyonghe
force-pushed
the
codex/autostart-tray-settings
branch
from
64b61e7 to
afe448d
Compare
zouyonghe
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider caching
DesktopSettingsin app state (and updating it when toggles change) instead of callingdesktop_settings::read_desktop_settingson every window event and tray interaction, to avoid repeated filesystem I/O on hot paths likeCloseRequestedandFocusedevents. - In
configure_window_events, the close-to-tray flag is recomputed from disk on each event; you could instead read it once during setup and pass it viastateor a shared struct to reduce redundant work and keep behavior consistent even if the settings file is modified externally mid-session.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider caching `DesktopSettings` in app state (and updating it when toggles change) instead of calling `desktop_settings::read_desktop_settings` on every window event and tray interaction, to avoid repeated filesystem I/O on hot paths like `CloseRequested` and `Focused` events.
- In `configure_window_events`, the close-to-tray flag is recomputed from disk on each event; you could instead read it once during setup and pass it via `state` or a shared struct to reduce redundant work and keep behavior consistent even if the settings file is modified externally mid-session.
## Individual Comments
### Comment 1
<location path="src-tauri/src/app_runtime.rs" line_range="29-34" />
<code_context>
fn configure_window_events(builder: Builder<tauri::Wry>) -> Builder<tauri::Wry> {
builder.on_window_event(|window, event| {
let is_quitting = window.app_handle().state::<BackendState>().is_quitting();
+ let desktop_settings = desktop_settings::read_desktop_settings(
+ runtime_paths::default_packaged_root_dir().as_deref(),
+ );
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid reading desktop settings from disk on every window event
This calls `desktop_settings::read_desktop_settings` on every window event, doing a sync filesystem read + JSON parse for each. For frequent events (e.g., focus/close) this is wasteful and can cause I/O jitter. Prefer caching `DesktopSettings` in app state (e.g., via `app.manage(...)` / `State`) and updating it when tray toggles change, so `configure_window_events` reads from memory instead of disk.
Suggested implementation:
```rust
fn configure_window_events(builder: Builder<tauri::Wry>) -> Builder<tauri::Wry> {
builder.on_window_event(|window, event| {
let is_quitting = window.app_handle().state::<BackendState>().is_quitting();
let desktop_settings = window
.app_handle()
.state::<desktop_settings::DesktopSettings>()
.clone();
let action = match &event {
```
To fully implement the caching approach and avoid disk reads:
1. Ensure `desktop_settings::DesktopSettings` is `Clone + Send + Sync + 'static` so it can be stored in Tauri state and cloned here.
2. During app setup (where you build the Tauri app), read the desktop settings once from disk and register them with `app.manage(desktop_settings::read_desktop_settings(runtime_paths::default_packaged_root_dir().as_deref()));`.
3. When tray toggles or any UI change modifies desktop settings, update the managed state instead of re-reading from disk (e.g., by storing them in a `RwLock` / `Mutex` inside the managed struct and mutating that).
4. If your project already uses a dedicated state struct (e.g., `BackendState`) for configuration, you may instead store `DesktopSettings` inside that struct and adjust the code above to fetch it via `BackendState` rather than a separate `DesktopSettings` state.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
zouyonghe
force-pushed
the
codex/autostart-tray-settings
branch
from
afe448d to
018290e
Compare
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Desktop settings are read from disk in several hot paths (e.g., on every window event and multiple tray handlers); consider caching
DesktopSettingsin application state and updating that cache when toggles change to avoid repeated file IO and JSON parsing in the event loop.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Desktop settings are read from disk in several hot paths (e.g., on every window event and multiple tray handlers); consider caching `DesktopSettings` in application state and updating that cache when toggles change to avoid repeated file IO and JSON parsing in the event loop.
## Individual Comments
### Comment 1
<location path="src-tauri/src/desktop_settings.rs" line_range="30" />
<code_context>
+ CloseToTray,
+}
+
+#[derive(Debug, Default, Serialize, Deserialize)]
+struct DesktopSettingsState {
+ #[serde(rename = "launchAtLogin")]
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing the Option-based state representation with concrete booleans using serde defaults and a From impl to simplify DesktopSettings handling.
You can simplify the state/setting split without losing any behavior by removing the `Option` indirection and letting `serde` handle defaults directly.
Today you have:
```rust
#[derive(Debug, Default, Serialize, Deserialize)]
struct DesktopSettingsState {
#[serde(rename = "launchAtLogin")]
launch_at_login: Option<bool>,
#[serde(rename = "silentLaunch")]
silent_launch: Option<bool>,
#[serde(rename = "closeToTray")]
close_to_tray: Option<bool>,
#[serde(flatten)]
other: Map<String, Value>,
}
impl DesktopSettingsState {
fn settings(&self) -> DesktopSettings {
let defaults = DesktopSettings::default();
DesktopSettings {
launch_at_login: self.launch_at_login.unwrap_or(defaults.launch_at_login),
silent_launch: self.silent_launch.unwrap_or(defaults.silent_launch),
close_to_tray: self.close_to_tray.unwrap_or(defaults.close_to_tray),
}
}
}
```
You can collapse this to concrete booleans with field-level defaults and drop `settings()` entirely:
```rust
fn default_launch_at_login() -> bool {
false
}
fn default_silent_launch() -> bool {
false
}
fn default_close_to_tray() -> bool {
true
}
#[derive(Debug, Serialize, Deserialize)]
struct DesktopSettingsState {
#[serde(rename = "launchAtLogin", default = "default_launch_at_login")]
launch_at_login: bool,
#[serde(rename = "silentLaunch", default = "default_silent_launch")]
silent_launch: bool,
#[serde(rename = "closeToTray", default = "default_close_to_tray")]
close_to_tray: bool,
#[serde(flatten)]
other: Map<String, Value>,
}
impl Default for DesktopSettingsState {
fn default() -> Self {
Self {
launch_at_login: default_launch_at_login(),
silent_launch: default_silent_launch(),
close_to_tray: default_close_to_tray(),
other: Map::new(),
}
}
}
```
Then `read_desktop_settings` and `write_desktop_setting` can map `DesktopSettingsState` to `DesktopSettings` by simple field copy, without Option logic:
```rust
impl From<DesktopSettingsState> for DesktopSettings {
fn from(state: DesktopSettingsState) -> Self {
DesktopSettings {
launch_at_login: state.launch_at_login,
silent_launch: state.silent_launch,
close_to_tray: state.close_to_tray,
}
}
}
// read
match load_state(&state_path) {
Ok(state) => DesktopSettings::from(state),
Err(error) => {
crate::append_desktop_log(&error);
DesktopSettings::default()
}
}
// write (after save_state)
Ok(DesktopSettings::from(state))
```
This keeps:
- the same defaults (`close_to_tray = true`, others `false`),
- preservation of unknown fields via `other`,
- the public `DesktopSettings` API,
while removing the `Option`-based state representation and the extra `settings()` transformation, making the module a bit easier to follow.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
zouyonghe
force-pushed
the
codex/autostart-tray-settings
branch
from
018290e to
7760d62
Compare
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src-tauri/src/desktop_settings.rs" line_range="112-138" />
<code_context>
+ }
+}
+
+fn load_state(path: &Path) -> Result<DesktopSettingsState, String> {
+ let raw = match fs::read_to_string(path) {
+ Ok(raw) => raw,
+ Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
+ return Ok(DesktopSettingsState::default());
+ }
+ Err(error) => {
+ return Err(format!(
+ "Failed to read desktop settings state {}: {}",
+ path.display(),
+ error
+ ));
+ }
+ };
+
+ match serde_json::from_str::<DesktopSettingsState>(&raw) {
+ Ok(state) => Ok(state),
+ Err(error) => {
+ crate::append_desktop_log(&format!(
+ "failed to parse desktop settings state {}: {}. resetting state file",
+ path.display(),
+ error
+ ));
+ Ok(DesktopSettingsState::default())
+ }
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Resetting to defaults on parse error but not rewriting the file can cause repeated errors across runs.
Because the corrupted file is left untouched, every startup will re-trigger the same parse failure and log entry until something else overwrites the file. It would be better to persist the default state immediately (e.g., via `save_state` or by truncating/recreating the file) so subsequent reads succeed. If you don’t want to overwrite automatically, consider a one-time recovery strategy (e.g., renaming the bad file and creating a fresh state file).
```suggestion
fn load_state(path: &Path) -> Result<DesktopSettingsState, String> {
let raw = match fs::read_to_string(path) {
Ok(raw) => raw,
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
return Ok(DesktopSettingsState::default());
}
Err(error) => {
return Err(format!(
"Failed to read desktop settings state {}: {}",
path.display(),
error
));
}
};
match serde_json::from_str::<DesktopSettingsState>(&raw) {
Ok(state) => Ok(state),
Err(error) => {
crate::append_desktop_log(&format!(
"failed to parse desktop settings state {}: {}. resetting state file",
path.display(),
error
));
let default_state = DesktopSettingsState::default();
if let Err(save_error) = save_state(path, &default_state) {
crate::append_desktop_log(&format!(
"failed to persist reset desktop settings state {}: {}",
path.display(),
save_error
));
}
Ok(default_state)
}
}
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
zouyonghe
force-pushed
the
codex/autostart-tray-settings
branch
from
7760d62 to
e777d4f
Compare
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src-tauri/src/tray/menu_handler.rs" line_range="95" />
<code_context>
+ }
+}
+
+fn persist_bool_setting(
+ app_handle: &AppHandle,
+ key: desktop_settings::DesktopSettingKey,
</code_context>
<issue_to_address>
**issue (complexity):** Consider unifying the boolean-setting persistence and tray-update logic into a single helper used by all three toggle handlers, while keeping `set_checked_safe` as a thin primitive.
You can reduce complexity and unify behavior by:
1. Using a single helper for “persist bool setting + update cache + sync tray item + error recovery”.
2. Letting launch‑at‑login reuse that helper (only its autostart side effect is special).
3. Keeping `set_checked_safe` but not layering another helper on top of it for only two callsites.
For example, replace `persist_bool_setting` and the three toggle handlers with something like this:
```rust
fn persist_bool_setting_and_update_tray(
app_handle: &AppHandle,
key: desktop_settings::DesktopSettingKey,
new_value: bool,
previous_value: bool,
item: &tauri::menu::CheckMenuItem<tauri::Wry>,
item_name: &str,
) {
match desktop_settings::write_desktop_setting(
runtime_paths::default_packaged_root_dir().as_deref(),
key,
new_value,
) {
Ok(updated_settings) => {
app_handle
.state::<DesktopSettingsCache>()
.set(updated_settings);
set_checked_safe(item, new_value, item_name);
}
Err(error) => {
append_desktop_log(&format!(
"failed to persist {} setting: {}",
item_name, error
));
// revert tray checkmark on failure
set_checked_safe(item, previous_value, item_name);
}
}
}
```
Then all three handlers share the same pattern:
```rust
fn handle_silent_launch_toggle(app_handle: &AppHandle) {
let Some(tray_state) = app_handle.try_state::<TrayMenuState>() else { return; };
let current = app_handle.state::<DesktopSettingsCache>().get().silent_launch;
persist_bool_setting_and_update_tray(
app_handle,
desktop_settings::DesktopSettingKey::SilentLaunch,
!current,
current,
&tray_state.silent_launch_item,
actions::TRAY_MENU_SILENT_LAUNCH,
);
}
fn handle_close_to_tray_toggle(app_handle: &AppHandle) {
let Some(tray_state) = app_handle.try_state::<TrayMenuState>() else { return; };
let current = app_handle.state::<DesktopSettingsCache>().get().close_to_tray;
persist_bool_setting_and_update_tray(
app_handle,
desktop_settings::DesktopSettingKey::CloseToTray,
!current,
current,
&tray_state.close_to_tray_item,
actions::TRAY_MENU_CLOSE_TO_TRAY,
);
}
fn handle_launch_at_login_toggle(app_handle: &AppHandle) {
let Some(tray_state) = app_handle.try_state::<TrayMenuState>() else { return; };
let current_enabled = match app_handle.autolaunch().is_enabled() {
Ok(value) => value,
Err(error) => {
append_desktop_log(&format!(
"failed to read launch-at-login state, using cached setting: {error}"
));
app_handle
.state::<DesktopSettingsCache>()
.get()
.launch_at_login
}
};
let desired_enabled = !current_enabled;
let op_res = if desired_enabled {
app_handle.autolaunch().enable()
} else {
app_handle.autolaunch().disable()
};
if let Err(error) = op_res {
append_desktop_log(&format!(
"failed to {} launch at login: {}",
if desired_enabled { "enable" } else { "disable" },
error
));
// keep tray in sync with actual autostart state
set_checked_safe(
&tray_state.launch_at_login_item,
current_enabled,
actions::TRAY_MENU_LAUNCH_AT_LOGIN,
);
return;
}
// reuse the same persistence + cache + tray error recovery logic
persist_bool_setting_and_update_tray(
app_handle,
desktop_settings::DesktopSettingKey::LaunchAtLogin,
desired_enabled,
current_enabled,
&tray_state.launch_at_login_item,
actions::TRAY_MENU_LAUNCH_AT_LOGIN,
);
}
```
This keeps all current behavior:
- Launch‑at‑login still uses the autostart plugin and falls back to cached state.
- All three toggles share a single error‑recovery path for settings persistence and tray updates.
- The flow is flatter and more uniform: each handler is “compute desired value + (optional) side effect + call common helper”, without multiple overlapping abstractions.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/desktop_settings.rs" line_range="7" />
<code_context>
+use serde_json::{Map, Value};
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) struct DesktopSettings {
+ pub(crate) launch_at_login: bool,
+ pub(crate) silent_launch: bool,
</code_context>
<issue_to_address>
**issue (complexity):** Consider collapsing the duplicated `DesktopSettings`/`DesktopSettingsState` types and simplifying `DesktopSettingsCache`’s mutex handling to reduce indirection and branching.
You can trim a fair bit of indirection here without losing any behavior. Two focused areas that give you most of the win:
---
### 1) Collapse `DesktopSettings` and `DesktopSettingsState`
You essentially have the same three booleans in two structs plus a `From` impl, and duplicated defaults. You can keep the `flatten` unknown-field preservation while exposing the same public API by:
- Making `DesktopSettings` the serde type.
- Adding a private `other` map for unknown fields.
- Providing a helper to expose the “public view” (or just use `DesktopSettings` directly where you already do).
For example:
```rust
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) struct DesktopSettingsPublic {
pub(crate) launch_at_login: bool,
pub(crate) silent_launch: bool,
pub(crate) close_to_tray: bool,
}
fn default_launch_at_login() -> bool { false }
fn default_silent_launch() -> bool { false }
fn default_close_to_tray() -> bool { true }
#[derive(Debug, Serialize, Deserialize, Default)]
struct DesktopSettings {
#[serde(rename = "launchAtLogin", default = "default_launch_at_login")]
launch_at_login: bool,
#[serde(rename = "silentLaunch", default = "default_silent_launch")]
silent_launch: bool,
#[serde(rename = "closeToTray", default = "default_close_to_tray")]
close_to_tray: bool,
#[serde(flatten)]
other: Map<String, Value>,
}
impl DesktopSettings {
fn public(&self) -> DesktopSettingsPublic {
DesktopSettingsPublic {
launch_at_login: self.launch_at_login,
silent_launch: self.silent_launch,
close_to_tray: self.close_to_tray,
}
}
fn set(&mut self, key: DesktopSettingKey, value: bool) {
match key {
DesktopSettingKey::LaunchAtLogin => self.launch_at_login = value,
DesktopSettingKey::SilentLaunch => self.silent_launch = value,
DesktopSettingKey::CloseToTray => self.close_to_tray = value,
}
}
}
```
Then your read/write paths simplify to one type:
```rust
pub(crate) fn read_desktop_settings(root: Option<&Path>) -> DesktopSettingsPublic {
let Some(path) = crate::desktop_state::resolve_desktop_state_path(root) else {
return DesktopSettings::default().public();
};
match load_state(&path) {
Ok(state) => state.public(),
Err(error) => {
crate::append_desktop_log(&error);
DesktopSettings::default().public()
}
}
}
pub(crate) fn write_desktop_setting(
root: Option<&Path>,
key: DesktopSettingKey,
value: bool,
) -> Result<DesktopSettingsPublic, String> {
let Some(path) = crate::desktop_state::resolve_desktop_state_path(root) else {
let message =
"Desktop settings state path is unavailable; cannot persist setting.".to_string();
crate::append_desktop_log(&message);
return Err(message);
};
let mut state = load_state(&path)?;
state.set(key, value);
save_state(&path, &state)?;
Ok(state.public())
}
```
This removes:
- The `DesktopSettingsState` type.
- The `From<DesktopSettingsState> for DesktopSettings` impl.
- Duplicate `Default` impls and the need to keep them in sync.
---
### 2) Simplify `DesktopSettingsCache` poison handling
For a simple cache of three booleans, the custom poison handling (logging, `into_inner`, etc.) adds branching and code paths that don’t materially improve safety. If a panic occurs while holding the lock, continuing with a potentially inconsistent value is usually worse than just crashing.
You can use straightforward locking:
```rust
#[derive(Debug)]
pub(crate) struct DesktopSettingsCache {
settings: Mutex<DesktopSettingsPublic>,
}
impl DesktopSettingsCache {
pub(crate) fn new(settings: DesktopSettingsPublic) -> Self {
Self {
settings: Mutex::new(settings),
}
}
pub(crate) fn get(&self) -> DesktopSettingsPublic {
*self.settings.lock().unwrap()
}
pub(crate) fn set(&self, settings: DesktopSettingsPublic) {
*self.settings.lock().unwrap() = settings;
}
}
```
This keeps all current behavior (same cached value semantics) but removes the extra branches and logging around poisoned locks.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
|
感谢大佬的功能完成 |
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.
Summary
Fixes AstrBotDevs/AstrBot#8404.
Details
This adds three desktop-client settings directly to the existing tray menu: 开机自启, 静默启动, and 关闭界面自动最小化托盘. Silent launch hides the main window after setup while still starting the backend normally. Close-to-tray now preserves the previous default behavior when enabled, and when disabled the close button performs a graceful app exit instead of only destroying the webview window.
The new desktop_settings module stores these settings in desktop_state.json using camelCase fields and preserves unrelated state such as locale and updateChannel. Launch-at-login uses the Tauri autostart plugin and reads the OS autostart state for the tray checkbox when available.
Verification
macOS build artifacts produced locally:
Summary by Sourcery
Introduce configurable desktop startup and tray behaviors, including launch-at-login, silent launch, and close-to-tray options, wired through the tray menu, persisted desktop settings, and updated window lifecycle handling.
New Features:
Enhancements:
Build:
Tests: