Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 130 additions & 4 deletions rust/frontend/src/models/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,11 @@ use crate::models::{with_persist_mut, with_persist_read};
use cxx_qt::{CxxQtType, Initialize};
use cxx_qt_lib::{QString, QStringList};
use std::pin::Pin;
use std::path::Path;
use tracing::warn;
use zaparoo_core::config::{load_config, save_settings_mirror, Config, SettingsMirror};
use zaparoo_core::persist::{self, SettingsState};
use zaparoo_core::platform_paths::config_file_path;
use zaparoo_core::platform_paths::{config_file_path, themes_dir_path};
use zaparoo_core::runtime;

/// Curated `MiSTer` resolution choices. Order is the left/right cycle
Expand All @@ -88,6 +89,8 @@ const LANGUAGES: &[&str] = &[
"hi",
];
const DEFAULT_LANGUAGE: &str = "auto";
const BUILTIN_THEMES: &[&str] = &["default", "crt"];
const DEFAULT_THEME: &str = "default";
const BROWSE_LAYOUTS: &[&str] = &["grid", "list"];
const DEFAULT_BROWSE_LAYOUT: &str = "grid";
const BUTTON_LAYOUTS: &[&str] = &["a", "b", "c", "d"];
Expand Down Expand Up @@ -118,6 +121,9 @@ pub struct SettingsRust {
current_resolution: QString,
available_languages: QStringList,
current_language: QString,
available_themes: QStringList,
current_theme: QString,
themes_directory_url: QString,
available_browse_layouts: QStringList,
current_browse_layout: QString,
available_button_layouts: QStringList,
Expand Down Expand Up @@ -147,6 +153,9 @@ pub mod ffi {
#[qproperty(QString, current_resolution, READ, WRITE = set_resolution, NOTIFY)]
#[qproperty(QStringList, available_languages, READ, CONSTANT)]
#[qproperty(QString, current_language, READ, WRITE = set_language, NOTIFY)]
#[qproperty(QStringList, available_themes, READ, CONSTANT)]
#[qproperty(QString, current_theme, READ, WRITE = set_theme, NOTIFY)]
#[qproperty(QString, themes_directory_url, READ, CONSTANT)]
#[qproperty(QStringList, available_browse_layouts, READ, CONSTANT)]
#[qproperty(QString, current_browse_layout, READ, WRITE = set_browse_layout, NOTIFY)]
#[qproperty(QStringList, available_button_layouts, READ, CONSTANT)]
Expand All @@ -164,6 +173,9 @@ pub mod ffi {
#[qinvokable]
fn set_language(self: Pin<&mut Settings>, value: QString);

#[qinvokable]
fn set_theme(self: Pin<&mut Settings>, value: QString);

#[qinvokable]
fn set_browse_layout(self: Pin<&mut Settings>, value: QString);

Expand Down Expand Up @@ -192,7 +204,8 @@ impl Initialize for ffi::Settings {
let config_path = config_file_path();
let is_mister = runtime::current().is_mister();
let config = load_config(&config_path);
let merged = merge_settings(&snapshot, &config);
let available_themes_vec = discover_themes(is_mister);
let merged = merge_settings(&snapshot, &config, &available_themes_vec);
persist_if_changed(&snapshot, &merged);
mirror_settings_to_config(&config_path, &merged);
self.as_mut().rust_mut().is_mister = is_mister;
Expand All @@ -204,6 +217,10 @@ impl Initialize for ffi::Settings {
self.as_mut().rust_mut().current_resolution = QString::from(merged.resolution.as_str());
self.as_mut().rust_mut().available_languages = languages();
self.as_mut().rust_mut().current_language = QString::from(merged.language.as_str());
self.as_mut().rust_mut().available_themes = qstring_list(&available_themes_vec);
self.as_mut().rust_mut().current_theme = QString::from(merged.theme.as_str());
self.as_mut().rust_mut().themes_directory_url =
QString::from(path_to_directory_url(&themes_dir_path()).as_str());
self.as_mut().rust_mut().available_browse_layouts = browse_layouts();
self.as_mut().rust_mut().current_browse_layout =
QString::from(merged.browse_layout.as_str());
Expand Down Expand Up @@ -250,6 +267,22 @@ impl ffi::Settings {
self.as_mut().current_language_changed();
}

#[allow(
clippy::needless_pass_by_value,
reason = "cxx-qt qinvokable signature requires QString by value"
)]
fn set_theme(mut self: Pin<&mut Self>, value: QString) {
let available = self.available_themes.iter().map(String::from).collect::<Vec<_>>();
let value_str = normalize_theme(&value.to_string(), &available);
if self.current_theme.to_string() == value_str {
return;
}
let snapshot = persist_settings(|s| s.theme.clone_from(&value_str));
mirror_settings_to_config(&config_file_path(), &snapshot.settings);
self.as_mut().rust_mut().current_theme = QString::from(value_str.as_str());
self.as_mut().current_theme_changed();
}

#[allow(
clippy::needless_pass_by_value,
reason = "cxx-qt qinvokable signature requires QString by value"
Expand Down Expand Up @@ -349,12 +382,13 @@ fn persist_if_changed(current: &SettingsState, merged: &SettingsState) {
persist::save(&snapshot);
}

fn mirror_settings_to_config(config_path: &std::path::Path, settings: &SettingsState) {
fn mirror_settings_to_config(config_path: &Path, settings: &SettingsState) {
if let Err(e) = save_settings_mirror(
config_path,
SettingsMirror {
resolution: settings.resolution.as_str(),
language: settings.language.as_str(),
theme: settings.theme.as_str(),
browse_layout: settings.browse_layout.as_str(),
button_layout: settings.button_layout.as_str(),
mouse_enabled: settings.mouse_enabled,
Expand All @@ -370,14 +404,22 @@ fn mirror_settings_to_config(config_path: &std::path::Path, settings: &SettingsS
}
}

fn merge_settings(snapshot: &SettingsState, config: &Config) -> SettingsState {
fn merge_settings(snapshot: &SettingsState, config: &Config, available_themes: &[String]) -> SettingsState {
SettingsState {
resolution: if config.video_explicit {
format!("{}x{}", config.video_width, config.video_height)
} else {
String::new()
},
language: normalize_language(&config.language).to_string(),
theme: normalize_theme(
config
.settings
.theme
.as_deref()
.unwrap_or(snapshot.theme.as_str()),
available_themes,
),
browse_layout: normalize_browse_layout(
config
.settings
Expand Down Expand Up @@ -416,6 +458,90 @@ fn merge_settings(snapshot: &SettingsState, config: &Config) -> SettingsState {
}
}

fn qstring_list(values: &[String]) -> QStringList {
let mut list = QStringList::default();
for value in values {
list.append(QString::from(value.as_str()));
}
list
}

fn discover_themes(is_mister: bool) -> Vec<String> {
let mut themes = BUILTIN_THEMES
.iter()
.map(|value| (*value).to_string())
.collect::<Vec<_>>();
if is_mister {
return themes;
}

let dir = themes_dir_path();
let Ok(entries) = std::fs::read_dir(&dir) else {
return themes;
};

let mut external = entries
.filter_map(Result::ok)
.filter_map(|entry| {
let path = entry.path();
if path.extension().and_then(|ext| ext.to_str()) != Some("json") {
return None;
}
path.file_stem()
.and_then(|stem| stem.to_str())
.map(str::trim)
.filter(|stem| !stem.is_empty())
.map(ToOwned::to_owned)
})
.filter(|theme| !themes.iter().any(|builtin| builtin == theme))
.collect::<Vec<_>>();
external.sort_unstable();
themes.extend(external);
themes
}

fn normalize_theme(value: &str, available_themes: &[String]) -> String {
let trimmed = value.trim();
if trimmed.is_empty() {
return DEFAULT_THEME.to_string();
}
if available_themes.iter().any(|theme| theme == trimmed) {
return trimmed.to_string();
}
DEFAULT_THEME.to_string()
}

fn path_to_directory_url(path: &Path) -> String {
let mut url = path_to_file_url(path);
if !url.ends_with('/') {
url.push('/');
}
url
}

fn path_to_file_url(path: &Path) -> String {
let absolute = if path.is_absolute() {
path.to_path_buf()
} else {
std::env::current_dir()
.unwrap_or_else(|_| ".".into())
.join(path)
};
let path_text = absolute.to_string_lossy();
let mut encoded = String::with_capacity(path_text.len() + 8);
for byte in path_text.as_bytes() {
match byte {
b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' | b'/' => {
encoded.push(char::from(*byte));
}
_ => {
let _ = std::fmt::Write::write_fmt(&mut encoded, format_args!("%{:02X}", byte));
}
}
}
format!("file://{encoded}")
Comment on lines +514 to +542
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python - <<'PY'
from pathlib import PureWindowsPath

def current_encoder(path_text: str) -> str:
    encoded = []
    for b in path_text.encode():
        if (48 <= b <= 57) or (65 <= b <= 90) or (97 <= b <= 122) or b in b"-_.~/":
            encoded.append(chr(b))
        else:
            encoded.append(f"%{b:02X}")
    return "file://" + "".join(encoded)

sample = r"C:\themes\crt theme"
print("current :", current_encoder(sample))
print("expected:", PureWindowsPath(sample).as_uri())
PY

Repository: ZaparooProject/zaparoo-frontend

Length of output: 160


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "fn path_to_directory_url|fn path_to_file_url" rust/frontend/src/models/settings.rs
python - <<'PY'
import pathlib, re
p = pathlib.Path("rust/frontend/src/models/settings.rs")
text = p.read_text(encoding="utf-8")
m = re.search(r"fn path_to_file_url\\([^\\)]*\\) -> String\\s*\\{([\\s\\S]*?)\\n\\}", text)
print("found path_to_file_url:", bool(m))
PY
sed -n '500,560p' rust/frontend/src/models/settings.rs

Repository: ZaparooProject/zaparoo-frontend

Length of output: 1802


Use a platform-aware file:// URL builder for themes_directory_url.
path_to_file_url() manually percent-encodes the absolute path and returns file://{encoded}; on Windows this produces an incorrect local-file URI shape (e.g., file://C%3A%5C... instead of file:///C:/...), so Qt round-tripping will fail. Replace this with a dedicated “file path → local file URI” helper and add tests for Windows paths (drive letters/backslashes). [lines 514-542]

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/frontend/src/models/settings.rs` around lines 514 - 542,
path_to_file_url and path_to_directory_url currently build file:// URIs by
manual percent-encoding which yields incorrect forms on Windows (e.g.
file://C%3A%5C...) — replace this logic in path_to_file_url with a
platform-aware conversion that: resolves an absolute Path, converts backslashes
to forward slashes on Windows, ensures the correct authority/triple-slash prefix
(file:/// for local absolute paths with drive letters), and percent-encodes only
reserved characters (use existing std::path or url crate helper if available, or
implement per-platform rules), then have path_to_directory_url call the
corrected path_to_file_url and append the trailing slash if missing; add unit
tests covering POSIX and Windows-style inputs (drive-letter + backslashes) to
validate produced URIs round-trip with Qt expectations.

}

fn curated_resolutions() -> QStringList {
let mut list = QStringList::default();
for r in MISTER_RESOLUTIONS {
Expand Down
14 changes: 14 additions & 0 deletions rust/zaparoo-core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub struct Config {

#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct SettingsConfig {
pub theme: Option<String>,
pub browse_layout: Option<String>,
pub button_layout: Option<String>,
pub mouse_enabled: Option<bool>,
Expand All @@ -55,6 +56,7 @@ pub struct SettingsConfig {
pub struct SettingsMirror<'a> {
pub resolution: &'a str,
pub language: &'a str,
pub theme: &'a str,
pub browse_layout: &'a str,
pub button_layout: &'a str,
pub mouse_enabled: bool,
Expand Down Expand Up @@ -131,6 +133,7 @@ struct RawInput {

#[derive(Deserialize, Default)]
struct RawSettings {
theme: Option<String>,
browse_layout: Option<String>,
button_layout: Option<String>,
mouse_enabled: Option<bool>,
Expand Down Expand Up @@ -201,6 +204,7 @@ pub fn load_config(path: &Path) -> Config {
cfg.key_to_action = input_actions::invert(&merged);
}
cfg.settings = SettingsConfig {
theme: raw.settings.theme.map(|value| value.trim().to_string()),
browse_layout: raw
.settings
.browse_layout
Expand Down Expand Up @@ -273,6 +277,10 @@ pub fn save_settings_mirror(path: &Path, mirror: SettingsMirror<'_>) -> Result<(
path.display()
));
};
settings.insert(
"theme".into(),
toml::Value::String(mirror.theme.trim().to_string()),
);
settings.insert(
"browse_layout".into(),
toml::Value::String(mirror.browse_layout.trim().to_string()),
Expand Down Expand Up @@ -600,6 +608,7 @@ mod tests {
SettingsMirror {
resolution: "1280x720",
language: "it_IT",
theme: "crt",
browse_layout: "list",
button_layout: "b",
mouse_enabled: false,
Expand All @@ -614,6 +623,7 @@ mod tests {
assert_eq!(cfg.video_width, 1280);
assert_eq!(cfg.video_height, 720);
assert!(cfg.video_explicit);
assert_eq!(cfg.settings.theme.as_deref(), Some("crt"));
assert_eq!(cfg.settings.browse_layout.as_deref(), Some("list"));
assert_eq!(cfg.settings.button_layout.as_deref(), Some("b"));
assert_eq!(cfg.settings.mouse_enabled, Some(false));
Expand All @@ -632,6 +642,7 @@ mod tests {
SettingsMirror {
resolution: "1280x720",
language: "en",
theme: "default",
browse_layout: "grid",
button_layout: "a",
mouse_enabled: true,
Expand All @@ -648,6 +659,7 @@ mod tests {
assert_eq!(cfg.core_endpoint, "ws://example.com/api");
assert_eq!(cfg.video_width, 1280);
assert_eq!(cfg.video_height, 720);
assert_eq!(cfg.settings.theme.as_deref(), Some("default"));
assert_eq!(cfg.settings.browse_layout.as_deref(), Some("grid"));
assert_eq!(cfg.settings.button_layout.as_deref(), Some("a"));
assert_eq!(cfg.settings.mouse_enabled, Some(true));
Expand All @@ -664,6 +676,7 @@ mod tests {
SettingsMirror {
resolution: "",
language: "",
theme: "crt",
browse_layout: "list",
button_layout: "c",
mouse_enabled: false,
Expand All @@ -675,6 +688,7 @@ mod tests {
.expect("save");
let written = std::fs::read_to_string(f.path()).expect("read");
assert!(written.contains("language = \"auto\""));
assert!(written.contains("theme = \"crt\""));
assert!(written.contains("browse_layout = \"list\""));
assert!(written.contains("button_layout = \"c\""));
assert!(written.contains("mouse_enabled = false"));
Expand Down
8 changes: 8 additions & 0 deletions rust/zaparoo-core/src/persist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ pub struct FavoritesState {
pub struct SettingsState {
pub resolution: String,
pub language: String,
#[serde(default = "default_theme")]
pub theme: String,
#[serde(default = "default_browse_layout")]
pub browse_layout: String,
#[serde(default = "default_button_layout")]
Expand All @@ -116,6 +118,7 @@ impl Default for SettingsState {
Self {
resolution: String::new(),
language: String::new(),
theme: default_theme(),
browse_layout: default_browse_layout(),
button_layout: default_button_layout(),
mouse_enabled: default_mouse_enabled(),
Expand All @@ -126,6 +129,10 @@ impl Default for SettingsState {
}
}

fn default_theme() -> String {
"default".into()
}

fn default_browse_layout() -> String {
"grid".into()
}
Expand Down Expand Up @@ -270,6 +277,7 @@ mod tests {
settings: SettingsState {
resolution: "1920x1080".into(),
language: "it_IT".into(),
theme: "crt".into(),
browse_layout: "list".into(),
button_layout: "b".into(),
mouse_enabled: false,
Expand Down
8 changes: 8 additions & 0 deletions rust/zaparoo-core/src/platform_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ pub fn state_file_path() -> PathBuf {
}
}

pub fn themes_dir_path() -> PathBuf {
std::env::current_exe()
.ok()
.and_then(|path| path.parent().map(std::path::Path::to_path_buf))
.unwrap_or_else(|| PathBuf::from("."))
.join("themes")
}

#[cfg(test)]
mod tests {
#![allow(
Expand Down
4 changes: 3 additions & 1 deletion src/ui/app/Main.qml
Original file line number Diff line number Diff line change
Expand Up @@ -1292,7 +1292,9 @@ MainLayout {
if (selectedId !== Browse.Settings.current_language)
root.stageSettingRestart(fieldId, selectedId);
return;
} else if (fieldId === "browseLayout")
} else if (fieldId === "theme")
Browse.Settings.set_theme(selectedId);
else if (fieldId === "browseLayout")
Browse.Settings.set_browse_layout(selectedId);
else if (fieldId === "buttonLayout")
Browse.Settings.set_button_layout(selectedId);
Expand Down
Loading
Loading