[AI] Refactor mirror change detection and configuration writing#34
[AI] Refactor mirror change detection and configuration writing#34Zeehan2005 wants to merge 1 commit intoapoint123:mainfrom
Conversation
Summary of ChangesHello @Zeehan2005, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在优化 AMLL 核心配置文件的管理方式,确保应用程序设置与实际配置文件之间的一致性。通过调整配置写入逻辑,现在无论镜像设置是否发生变化,核心配置文件都会在每次保存时更新,同时明确了 mirror_changed 变量的单一职责,使其仅用于触发用户界面提示。 Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
该 PR 重构了 AMLL 镜像变更检测与核心配置写入逻辑,使应用每次保存设置时都会写入 AMLL 核心配置文件(%APPDATA%\lyrics-helper\amll_config.json),并将 mirror_changed 的职责收敛为仅控制重启提示。
Changes:
- 将
mirror_changed改为仅根据旧/新 mirror 是否不同来决定是否提示重启 - 每次保存设置都序列化并写入
amll_config.json,不再依赖 mirror 是否变化 - 保留序列化/写入失败时的错误日志输出
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let old_settings = self.app_settings.lock().unwrap(); | ||
| let new_mirror = &settings.amll_mirror; | ||
| if &old_settings.amll_mirror != new_mirror { | ||
| let core_config = CoreAmllConfig { | ||
| mirror: new_mirror.clone().into(), | ||
| }; | ||
| match serde_json::to_string_pretty(&core_config) { | ||
| Ok(json_string) => { | ||
| if let Ok(config_path) = | ||
| lyrics_helper_rs::config::get_config_file_path( | ||
| "amll_config.json", | ||
| ) | ||
| { | ||
| if let Err(e) = std::fs::write(&config_path, json_string) { | ||
| error!("[Settings] 写入 amll_config.json 失败: {}", e); | ||
| } else { | ||
| mirror_changed = true; | ||
| } | ||
| } else { | ||
| error!("[Settings] 无法获取 amll_config.json 的路径"); | ||
| mirror_changed = &old_settings.amll_mirror != new_mirror; | ||
|
|
||
| let core_config = CoreAmllConfig { | ||
| mirror: new_mirror.clone().into(), | ||
| }; | ||
| match serde_json::to_string_pretty(&core_config) { | ||
| Ok(json_string) => { | ||
| if let Ok(config_path) = | ||
| lyrics_helper_rs::config::get_config_file_path( | ||
| "amll_config.json", | ||
| ) | ||
| { | ||
| if let Err(e) = std::fs::write(&config_path, json_string) { | ||
| error!("[Settings] 写入 amll_config.json 失败: {}", e); | ||
| } |
There was a problem hiding this comment.
这里在持有 self.app_settings 的 mutex guard 时做了 JSON 序列化与 std::fs::write 的磁盘 I/O。此次改动后每次保存都会写文件,会显著拉长锁持有时间,可能阻塞其它读取/更新设置的路径。建议只在锁内拷贝出旧 mirror 值并计算 mirror_changed,随后尽早 drop guard,再进行序列化/写入。
| mirror_changed = &old_settings.amll_mirror != new_mirror; | ||
|
|
||
| let core_config = CoreAmllConfig { | ||
| mirror: new_mirror.clone().into(), | ||
| }; | ||
| match serde_json::to_string_pretty(&core_config) { | ||
| Ok(json_string) => { | ||
| if let Ok(config_path) = | ||
| lyrics_helper_rs::config::get_config_file_path( | ||
| "amll_config.json", | ||
| ) | ||
| { | ||
| if let Err(e) = std::fs::write(&config_path, json_string) { | ||
| error!("[Settings] 写入 amll_config.json 失败: {}", e); | ||
| } | ||
| } else { | ||
| error!("[Settings] 无法获取 amll_config.json 的路径"); | ||
| } | ||
| Err(e) => { | ||
| error!("[Settings] 序列化核心库 AMLL 配置失败: {}", e); | ||
| } | ||
| } | ||
| Err(e) => { | ||
| error!("[Settings] 序列化核心库 AMLL 配置失败: {}", e); | ||
| } |
There was a problem hiding this comment.
mirror_changed 现在在写入 amll_config.json 之前就基于 mirror 差异直接置为 true,后续会触发“需要重新启动才能生效”的提示;但如果序列化或写文件失败(这里只记录了 error 日志),用户仍可能看到该提示,实际配置并未生效。建议区分“mirror 是否变化”和“核心配置是否成功写入”,仅在写入成功时提示需要重启,或在失败时给出失败提示/返回 Warning。
| match serde_json::to_string_pretty(&core_config) { | ||
| Ok(json_string) => { | ||
| if let Ok(config_path) = | ||
| lyrics_helper_rs::config::get_config_file_path( | ||
| "amll_config.json", | ||
| ) | ||
| { | ||
| if let Err(e) = std::fs::write(&config_path, json_string) { | ||
| error!("[Settings] 写入 amll_config.json 失败: {}", e); | ||
| } | ||
| } else { | ||
| error!("[Settings] 无法获取 amll_config.json 的路径"); | ||
| } | ||
| Err(e) => { | ||
| error!("[Settings] 序列化核心库 AMLL 配置失败: {}", e); | ||
| } | ||
| } | ||
| Err(e) => { | ||
| error!("[Settings] 序列化核心库 AMLL 配置失败: {}", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
这部分代码功能正确,但存在多层嵌套的 match 和 if let,导致可读性稍差。可以将其重构为一个独立的闭包,使用卫语句(guard clauses)提早返回,使代码结构更扁平、更清晰。
let write_amll_config = || {
let json_string = match serde_json::to_string_pretty(&core_config) {
Ok(s) => s,
Err(e) => {
error!("[Settings] 序列化核心库 AMLL 配置失败: {}", e);
return;
}
};
let config_path = match lyrics_helper_rs::config::get_config_file_path("amll_config.json") {
Ok(p) => p,
Err(_) => {
error!("[Settings] 无法获取 amll_config.json 的路径");
return;
}
};
if let Err(e) = std::fs::write(&config_path, json_string) {
error!("[Settings] 写入 amll_config.json 失败: {}", e);
}
};
write_amll_config();
已调整为每次保存设置都会写入 AMLL 核心配置文件,确保 %APPDATA%\lyrics-helper\amll_config.json 始终与设置同步;mirror_changed只负责控制提示,而不再影响是否写入。改动在 app_handlers.rs:1016-1052。
如果你要验证效果,可以:
保存一次设置后检查 %APPDATA%\lyrics-helper\amll_config.json 是否生成并更新
重新加载提供商或重启应用观察日志是否走 GitHub