mirror of
https://github.com/outbackdingo/hyprcosmic.git
synced 2026-08-25 07:10:09 +00:00
Fix every clippy lint, including one real panic
Mostly mechanical -- writeln! for format strings ending in a newline, sort_by_key, slice::from_ref, &Path over &PathBuf, a stray &mut in a test. Two were worth more than the lint that found them. `parse_color` sliced `hex[i..i + 2]` after checking `hex.len()`. Both are byte counts, so a multi-byte character inside `rgb(...)` split a char boundary and panicked: `rgb(€abc)` is six bytes and aborted the compiler with "end byte index 2 is not a char boundary". A typo in a config file must produce a diagnostic, not a crash. Clippy did not see this -- it flagged the duplicated `rgb(`/`rgba(` arms as foldable into `?`, and folding them is what put the two length assumptions next to each other where the mismatch was visible. Now guarded by is_ascii, with a test that panics without the guard. `plan_verbatim` tripped too_many_arguments at 8. Three of them were the `&mut Vec<Action>`, `&mut Vec<Note>` and `&mut Vec<AssetError>` threaded through both plan helpers -- a Plan under construction, so `Draft` now names it and `finish()` owns the errors-are-fatal rule that was previously inline. 118 unit tests plus 4 integration tests pass; `cargo fmt --check` and `cargo clippy --all-targets` are both clean.
This commit is contained in:
+129
-49
@@ -43,7 +43,10 @@ pub enum AssetError {
|
||||
/// absolute path or a `..` component. Both would let extraction write
|
||||
/// outside `dest_root`, so this is refused unconditionally rather than
|
||||
/// sanitised — a theme directory is untrusted input.
|
||||
UnsafeArchiveEntry { archive: PathBuf, entry: PathBuf },
|
||||
UnsafeArchiveEntry {
|
||||
archive: PathBuf,
|
||||
entry: PathBuf,
|
||||
},
|
||||
NoHomeDirectory,
|
||||
}
|
||||
|
||||
@@ -158,6 +161,33 @@ pub struct Report {
|
||||
pub installed: Vec<PathBuf>,
|
||||
}
|
||||
|
||||
/// A `Plan` under construction, plus the errors found while building it.
|
||||
///
|
||||
/// The three collections travel together through every `plan_*` helper, so
|
||||
/// they are one parameter rather than three `&mut Vec`s. Errors accumulate
|
||||
/// instead of returning early: a theme with one unreadable file should still
|
||||
/// report what it would have done with the rest, the same way `resolve`
|
||||
/// collects diagnostics rather than stopping at the first.
|
||||
#[derive(Default)]
|
||||
struct Draft {
|
||||
actions: Vec<Action>,
|
||||
skipped: Vec<Note>,
|
||||
errors: Vec<AssetError>,
|
||||
}
|
||||
|
||||
impl Draft {
|
||||
fn finish(self) -> Result<Plan, Vec<AssetError>> {
|
||||
if self.errors.is_empty() {
|
||||
Ok(Plan {
|
||||
actions: self.actions,
|
||||
skipped: self.skipped,
|
||||
})
|
||||
} else {
|
||||
Err(self.errors)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Action {
|
||||
pub fn kind(&self) -> AssetKind {
|
||||
match self {
|
||||
@@ -280,9 +310,7 @@ impl Installer {
|
||||
theme_name: &str,
|
||||
overwrite: bool,
|
||||
) -> Result<Plan, Vec<AssetError>> {
|
||||
let mut actions = Vec::new();
|
||||
let mut skipped = Vec::new();
|
||||
let mut errors = Vec::new();
|
||||
let mut draft = Draft::default();
|
||||
|
||||
if let Some(source_dir) = source_dir {
|
||||
for (kind, prefix, dest_root) in [
|
||||
@@ -292,36 +320,32 @@ impl Installer {
|
||||
match find_tarball(source_dir, prefix) {
|
||||
Ok(Some(archive)) => {
|
||||
match self.plan_archive(kind, &archive, &dest_root, overwrite) {
|
||||
Ok(Some(action)) => actions.push(action),
|
||||
Ok(None) => skipped.push(Note {
|
||||
Ok(Some(action)) => draft.actions.push(action),
|
||||
Ok(None) => draft.skipped.push(Note {
|
||||
kind,
|
||||
path: archive,
|
||||
reason: SkipReason::AlreadyInstalled,
|
||||
}),
|
||||
Err(e) => errors.push(e),
|
||||
Err(e) => draft.errors.push(e),
|
||||
}
|
||||
}
|
||||
Ok(None) => {} // No tarball of this kind — not every theme ships both.
|
||||
Err(e) => errors.push(e),
|
||||
Err(e) => draft.errors.push(e),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
self.plan_wallpapers(theme_dir, theme_name, overwrite, &mut actions, &mut skipped, &mut errors);
|
||||
self.plan_wallpapers(theme_dir, theme_name, overwrite, &mut draft);
|
||||
|
||||
for (kind, filename) in [
|
||||
(AssetKind::Waybar, "waybar.theme"),
|
||||
(AssetKind::Rofi, "rofi.theme"),
|
||||
(AssetKind::Kitty, "kitty.theme"),
|
||||
] {
|
||||
self.plan_verbatim(theme_dir, kind, filename, overwrite, &mut actions, &mut skipped, &mut errors);
|
||||
self.plan_verbatim(theme_dir, kind, filename, overwrite, &mut draft);
|
||||
}
|
||||
|
||||
if errors.is_empty() {
|
||||
Ok(Plan { actions, skipped })
|
||||
} else {
|
||||
Err(errors)
|
||||
}
|
||||
draft.finish()
|
||||
}
|
||||
|
||||
fn plan_archive(
|
||||
@@ -354,9 +378,7 @@ impl Installer {
|
||||
theme_dir: &Path,
|
||||
theme_name: &str,
|
||||
overwrite: bool,
|
||||
actions: &mut Vec<Action>,
|
||||
skipped: &mut Vec<Note>,
|
||||
errors: &mut Vec<AssetError>,
|
||||
draft: &mut Draft,
|
||||
) {
|
||||
let wallpapers_dir = theme_dir.join("wallpapers");
|
||||
if !wallpapers_dir.is_dir() {
|
||||
@@ -371,7 +393,7 @@ impl Installer {
|
||||
let entries = match fs::read_dir(&wallpapers_dir) {
|
||||
Ok(e) => e,
|
||||
Err(e) => {
|
||||
errors.push(e.into());
|
||||
draft.errors.push(e.into());
|
||||
return;
|
||||
}
|
||||
};
|
||||
@@ -379,7 +401,7 @@ impl Installer {
|
||||
let entry = match entry {
|
||||
Ok(e) => e,
|
||||
Err(e) => {
|
||||
errors.push(e.into());
|
||||
draft.errors.push(e.into());
|
||||
continue;
|
||||
}
|
||||
};
|
||||
@@ -389,14 +411,14 @@ impl Installer {
|
||||
}
|
||||
let dest = dest_dir.join(entry.file_name());
|
||||
if !overwrite && dest.exists() {
|
||||
skipped.push(Note {
|
||||
draft.skipped.push(Note {
|
||||
kind: AssetKind::Wallpaper,
|
||||
path: src,
|
||||
reason: SkipReason::AlreadyInstalled,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
actions.push(Action::CopyWallpaper { src, dest });
|
||||
draft.actions.push(Action::CopyWallpaper { src, dest });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -406,9 +428,7 @@ impl Installer {
|
||||
kind: AssetKind,
|
||||
filename: &str,
|
||||
overwrite: bool,
|
||||
actions: &mut Vec<Action>,
|
||||
skipped: &mut Vec<Note>,
|
||||
errors: &mut Vec<AssetError>,
|
||||
draft: &mut Draft,
|
||||
) {
|
||||
let src = theme_dir.join(filename);
|
||||
if !src.is_file() {
|
||||
@@ -417,20 +437,20 @@ impl Installer {
|
||||
let text = match fs::read_to_string(&src) {
|
||||
Ok(t) => t,
|
||||
Err(e) => {
|
||||
errors.push(e.into());
|
||||
draft.errors.push(e.into());
|
||||
return;
|
||||
}
|
||||
};
|
||||
match split_hyde_header(&text, &self.home) {
|
||||
Some((dest, body)) => {
|
||||
if !overwrite && dest.exists() {
|
||||
skipped.push(Note {
|
||||
draft.skipped.push(Note {
|
||||
kind,
|
||||
path: src,
|
||||
reason: SkipReason::AlreadyInstalled,
|
||||
});
|
||||
} else {
|
||||
actions.push(Action::CopyVerbatim {
|
||||
draft.actions.push(Action::CopyVerbatim {
|
||||
kind,
|
||||
src,
|
||||
dest,
|
||||
@@ -438,7 +458,7 @@ impl Installer {
|
||||
});
|
||||
}
|
||||
}
|
||||
None => skipped.push(Note {
|
||||
None => draft.skipped.push(Note {
|
||||
kind,
|
||||
path: src,
|
||||
reason: SkipReason::NoDestinationHeader,
|
||||
@@ -520,7 +540,11 @@ fn find_tarball(dir: &Path, prefix: &str) -> Result<Option<PathBuf>, AssetError>
|
||||
/// entry first. Shared between `plan` (`write: false`, a pure read used only
|
||||
/// to name-check and reject unsafe archives early) and `apply`
|
||||
/// (`write: true`), so the safety check cannot drift between the two paths.
|
||||
fn walk_archive(archive_path: &Path, dest_root: &Path, write: bool) -> Result<Vec<PathBuf>, AssetError> {
|
||||
fn walk_archive(
|
||||
archive_path: &Path,
|
||||
dest_root: &Path,
|
||||
write: bool,
|
||||
) -> Result<Vec<PathBuf>, AssetError> {
|
||||
let file = fs::File::open(archive_path)?;
|
||||
let mut ar = tar::Archive::new(GzDecoder::new(file));
|
||||
let mut entries = Vec::new();
|
||||
@@ -578,7 +602,10 @@ fn walk_archive(archive_path: &Path, dest_root: &Path, write: bool) -> Result<Ve
|
||||
/// out of it. This is the hard security boundary — a theme directory is
|
||||
/// untrusted input.
|
||||
fn reject_unsafe_path(archive: &Path, entry: &Path) -> Result<(), AssetError> {
|
||||
let escapes = entry.is_absolute() || entry.components().any(|c| matches!(c, Component::ParentDir));
|
||||
let escapes = entry.is_absolute()
|
||||
|| entry
|
||||
.components()
|
||||
.any(|c| matches!(c, Component::ParentDir));
|
||||
if escapes {
|
||||
return Err(AssetError::UnsafeArchiveEntry {
|
||||
archive: archive.to_path_buf(),
|
||||
@@ -663,15 +690,24 @@ mod tests {
|
||||
Path::new("../../../../etc/passwd")
|
||||
));
|
||||
assert!(!stays_within_root(Path::new(""), Path::new("../escape")));
|
||||
assert!(!stays_within_root(Path::new("a"), Path::new("../../escape")));
|
||||
assert!(!stays_within_root(
|
||||
Path::new("a"),
|
||||
Path::new("../../escape")
|
||||
));
|
||||
// Exactly back to the root is fine; one further is not.
|
||||
assert!(stays_within_root(Path::new("a/b"), Path::new("../../c")));
|
||||
assert!(!stays_within_root(Path::new("a/b"), Path::new("../../../c")));
|
||||
assert!(!stays_within_root(
|
||||
Path::new("a/b"),
|
||||
Path::new("../../../c")
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_absolute_symlink_target_is_refused_however_it_is_spelled() {
|
||||
assert!(!stays_within_root(Path::new("a/b"), Path::new("/etc/passwd")));
|
||||
assert!(!stays_within_root(
|
||||
Path::new("a/b"),
|
||||
Path::new("/etc/passwd")
|
||||
));
|
||||
assert!(!stays_within_root(Path::new("a/b"), Path::new("/")));
|
||||
}
|
||||
|
||||
@@ -721,7 +757,7 @@ mod tests {
|
||||
let name_bytes = entry_path.as_bytes();
|
||||
header.as_mut_bytes()[..name_bytes.len()].copy_from_slice(name_bytes);
|
||||
header.set_cksum();
|
||||
builder.append(&mut header, data).unwrap();
|
||||
builder.append(&header, data).unwrap();
|
||||
|
||||
builder.into_inner().unwrap().finish().unwrap();
|
||||
path
|
||||
@@ -786,11 +822,18 @@ mod tests {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let theme_dir = tmp.path().join("theme");
|
||||
write(&theme_dir.join("wallpapers/bg.png"), "not really a png");
|
||||
write(&theme_dir.join("rofi.theme"), "$HOME/.config/rofi/theme.rasi\n* { main-bg: #000; }\n");
|
||||
write(
|
||||
&theme_dir.join("rofi.theme"),
|
||||
"$HOME/.config/rofi/theme.rasi\n* { main-bg: #000; }\n",
|
||||
);
|
||||
|
||||
let source_dir = tmp.path().join("Source");
|
||||
fs::create_dir_all(&source_dir).unwrap();
|
||||
make_tarball(&source_dir, "Gtk_Mocha.tar.gz", &[("Mocha/gtk.css", b"* {}")]);
|
||||
make_tarball(
|
||||
&source_dir,
|
||||
"Gtk_Mocha.tar.gz",
|
||||
&[("Mocha/gtk.css", b"* {}")],
|
||||
);
|
||||
|
||||
let home = tmp.path().join("home");
|
||||
let installer = Installer::with_paths(home.join(".local/share"), &home);
|
||||
@@ -799,7 +842,10 @@ mod tests {
|
||||
.expect("a well-formed theme must plan cleanly");
|
||||
|
||||
assert!(!plan.actions.is_empty());
|
||||
assert!(!home.exists(), "planning must not create anything under $HOME");
|
||||
assert!(
|
||||
!home.exists(),
|
||||
"planning must not create anything under $HOME"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -826,7 +872,10 @@ mod tests {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let theme_dir = tmp.path().join("theme");
|
||||
let body = "* {\n main-bg: #11111be6;\n main-fg: #cdd6f4ff;\n}\n";
|
||||
write(&theme_dir.join("rofi.theme"), &format!("$HOME/.config/rofi/theme.rasi\n{body}"));
|
||||
write(
|
||||
&theme_dir.join("rofi.theme"),
|
||||
&format!("$HOME/.config/rofi/theme.rasi\n{body}"),
|
||||
);
|
||||
|
||||
let home = tmp.path().join("home");
|
||||
let installer = Installer::with_paths(home.join(".local/share"), &home);
|
||||
@@ -877,12 +926,22 @@ mod tests {
|
||||
fs::create_dir_all(&theme_dir).unwrap();
|
||||
let source_dir = tmp.path().join("Source");
|
||||
fs::create_dir_all(&source_dir).unwrap();
|
||||
make_tarball(&source_dir, "Gtk_Mocha.tar.gz", &[("Mocha/gtk-3.0/gtk.css", b"* {}")]);
|
||||
make_tarball(&source_dir, "Icon_Tela.tar.gz", &[("Tela/index.theme", b"[Icon Theme]")]);
|
||||
make_tarball(
|
||||
&source_dir,
|
||||
"Gtk_Mocha.tar.gz",
|
||||
&[("Mocha/gtk-3.0/gtk.css", b"* {}")],
|
||||
);
|
||||
make_tarball(
|
||||
&source_dir,
|
||||
"Icon_Tela.tar.gz",
|
||||
&[("Tela/index.theme", b"[Icon Theme]")],
|
||||
);
|
||||
|
||||
let home = tmp.path().join("home");
|
||||
let installer = Installer::with_paths(home.join(".local/share"), &home);
|
||||
let plan = installer.plan(&theme_dir, Some(&source_dir), "Mocha", false).unwrap();
|
||||
let plan = installer
|
||||
.plan(&theme_dir, Some(&source_dir), "Mocha", false)
|
||||
.unwrap();
|
||||
installer.apply(&plan).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
@@ -902,23 +961,35 @@ mod tests {
|
||||
fs::create_dir_all(&theme_dir).unwrap();
|
||||
let source_dir = tmp.path().join("Source");
|
||||
fs::create_dir_all(&source_dir).unwrap();
|
||||
make_tarball(&source_dir, "Gtk_Mocha.tar.gz", &[("Mocha/gtk.css", b"new")]);
|
||||
make_tarball(
|
||||
&source_dir,
|
||||
"Gtk_Mocha.tar.gz",
|
||||
&[("Mocha/gtk.css", b"new")],
|
||||
);
|
||||
|
||||
let home = tmp.path().join("home");
|
||||
// Simulate a theme already installed under the name the tarball uses.
|
||||
write(&home.join(".themes/Mocha/gtk.css"), "old");
|
||||
|
||||
let installer = Installer::with_paths(home.join(".local/share"), &home);
|
||||
let plan = installer.plan(&theme_dir, Some(&source_dir), "Mocha", false).unwrap();
|
||||
let plan = installer
|
||||
.plan(&theme_dir, Some(&source_dir), "Mocha", false)
|
||||
.unwrap();
|
||||
|
||||
assert!(plan.actions.is_empty(), "already-installed theme must not be re-planned");
|
||||
assert!(
|
||||
plan.actions.is_empty(),
|
||||
"already-installed theme must not be re-planned"
|
||||
);
|
||||
assert_eq!(plan.skipped.len(), 1);
|
||||
assert_eq!(plan.skipped[0].reason, SkipReason::AlreadyInstalled);
|
||||
|
||||
// Confirm the skip is honoured all the way through apply, and the
|
||||
// existing file is left untouched.
|
||||
installer.apply(&plan).unwrap();
|
||||
assert_eq!(fs::read_to_string(home.join(".themes/Mocha/gtk.css")).unwrap(), "old");
|
||||
assert_eq!(
|
||||
fs::read_to_string(home.join(".themes/Mocha/gtk.css")).unwrap(),
|
||||
"old"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -928,17 +999,26 @@ mod tests {
|
||||
fs::create_dir_all(&theme_dir).unwrap();
|
||||
let source_dir = tmp.path().join("Source");
|
||||
fs::create_dir_all(&source_dir).unwrap();
|
||||
make_tarball(&source_dir, "Gtk_Mocha.tar.gz", &[("Mocha/gtk.css", b"new")]);
|
||||
make_tarball(
|
||||
&source_dir,
|
||||
"Gtk_Mocha.tar.gz",
|
||||
&[("Mocha/gtk.css", b"new")],
|
||||
);
|
||||
|
||||
let home = tmp.path().join("home");
|
||||
write(&home.join(".themes/Mocha/gtk.css"), "old");
|
||||
|
||||
let installer = Installer::with_paths(home.join(".local/share"), &home);
|
||||
let plan = installer.plan(&theme_dir, Some(&source_dir), "Mocha", true).unwrap();
|
||||
let plan = installer
|
||||
.plan(&theme_dir, Some(&source_dir), "Mocha", true)
|
||||
.unwrap();
|
||||
assert_eq!(plan.actions.len(), 1);
|
||||
|
||||
installer.apply(&plan).unwrap();
|
||||
assert_eq!(fs::read_to_string(home.join(".themes/Mocha/gtk.css")).unwrap(), "new");
|
||||
assert_eq!(
|
||||
fs::read_to_string(home.join(".themes/Mocha/gtk.css")).unwrap(),
|
||||
"new"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -59,9 +59,9 @@ fn reject_unknown(args: &[String], flags: &[&str], valued: &[&str]) -> Result<()
|
||||
let a = &args[i];
|
||||
if valued.contains(&a.as_str()) {
|
||||
i += 2;
|
||||
} else if flags.contains(&a.as_str()) {
|
||||
i += 1;
|
||||
} else if a == "-h" || a == "--help" {
|
||||
} else if flags.contains(&a.as_str()) || a == "-h" || a == "--help" {
|
||||
// `--help` is accepted by every subcommand, so callers do not have
|
||||
// to list it; `main` has already acted on it by this point.
|
||||
i += 1;
|
||||
} else if let Some(name) = a.strip_prefix("--") {
|
||||
return Err(format!("error: unknown option `--{name}`"));
|
||||
@@ -139,7 +139,7 @@ fn main() -> ExitCode {
|
||||
}
|
||||
}
|
||||
|
||||
fn run(config_path: &PathBuf, diff_only: bool) -> Result<String, String> {
|
||||
fn run(config_path: &Path, diff_only: bool) -> Result<String, String> {
|
||||
let emitter = Emitter::from_env().map_err(|e| format!("error: {e}\n"))?;
|
||||
|
||||
// Through `watch::compile` rather than parse/resolve/plan inline, because
|
||||
@@ -175,7 +175,9 @@ fn run(config_path: &PathBuf, diff_only: bool) -> Result<String, String> {
|
||||
return Ok(out);
|
||||
}
|
||||
|
||||
let written = emitter.apply(&planned).map_err(|e| format!("error: {e}\n"))?;
|
||||
let written = emitter
|
||||
.apply(&planned)
|
||||
.map_err(|e| format!("error: {e}\n"))?;
|
||||
Ok(format!(
|
||||
"Applied {written} change(s) to {}.",
|
||||
emitter.root().display()
|
||||
|
||||
+70
-20
@@ -155,15 +155,21 @@ fn eval_arith(input: &str) -> String {
|
||||
/// trade-off, and HyDE themes write colours as `rgba(...)`, so nothing is lost.
|
||||
fn parse_color(raw: &str) -> Option<(u8, u8, u8, u8)> {
|
||||
let s = raw.trim();
|
||||
let hex = if let Some(inner) = s.strip_prefix("rgba(").and_then(|s| s.strip_suffix(')')) {
|
||||
inner.trim().to_string()
|
||||
} else if let Some(inner) = s.strip_prefix("rgb(").and_then(|s| s.strip_suffix(')')) {
|
||||
inner.trim().to_string()
|
||||
} else {
|
||||
return None;
|
||||
};
|
||||
// Both spellings take the same body; the alpha pair is optional either
|
||||
// way, so `rgb(rrggbbaa)` and `rgba(rrggbb)` are accepted too rather than
|
||||
// rejected on a technicality.
|
||||
let inner = s
|
||||
.strip_prefix("rgba(")
|
||||
.or_else(|| s.strip_prefix("rgb("))
|
||||
.and_then(|s| s.strip_suffix(')'))?;
|
||||
|
||||
let hex = hex.trim_start_matches('#');
|
||||
let hex = inner.trim().trim_start_matches('#');
|
||||
// `len` and the slicing below are both in bytes, so a multi-byte character
|
||||
// would make `hex[i..i + 2]` split a char boundary and panic. A config
|
||||
// typo must not crash the compiler.
|
||||
if !hex.is_ascii() {
|
||||
return None;
|
||||
}
|
||||
let byte = |i: usize| u8::from_str_radix(&hex[i..i + 2], 16).ok();
|
||||
match hex.len() {
|
||||
6 => Some((byte(0)?, byte(2)?, byte(4)?, 255)),
|
||||
@@ -217,7 +223,10 @@ fn check_range(v: &Value, range: Option<Range>, span: Span) -> Result<(), Diagno
|
||||
};
|
||||
if n < r.min || n > r.max {
|
||||
return Err(Diagnostic {
|
||||
message: format!("value {n} is outside the allowed range {}..={}", r.min, r.max),
|
||||
message: format!(
|
||||
"value {n} is outside the allowed range {}..={}",
|
||||
r.min, r.max
|
||||
),
|
||||
span,
|
||||
help: None,
|
||||
});
|
||||
@@ -299,7 +308,14 @@ pub fn resolve(ast: &Ast) -> Result<Resolved, Vec<Diagnostic>> {
|
||||
continue;
|
||||
}
|
||||
|
||||
record(entry, value, raw_value.span, &mut projected, &mut whole, &mut diags);
|
||||
record(
|
||||
entry,
|
||||
value,
|
||||
raw_value.span,
|
||||
&mut projected,
|
||||
&mut whole,
|
||||
&mut diags,
|
||||
);
|
||||
}
|
||||
|
||||
if !diags.is_empty() {
|
||||
@@ -405,9 +421,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn binds_fold_into_one_write_against_the_shortcuts_custom_key() {
|
||||
let r = resolved(
|
||||
"bind = SUPER, D, exec, rofi -show drun\nbind = SUPER, Q, killactive\n",
|
||||
);
|
||||
let r = resolved("bind = SUPER, D, exec, rofi -show drun\nbind = SUPER, Q, killactive\n");
|
||||
let w: Vec<_> = r
|
||||
.writes
|
||||
.iter()
|
||||
@@ -420,8 +434,14 @@ mod tests {
|
||||
let WriteKind::Verbatim(ron) = &w[0].kind else {
|
||||
panic!("expected verbatim RON, got {:?}", w[0].kind);
|
||||
};
|
||||
assert!(ron.contains(r#"(modifiers: [Super], key: "d"): Spawn("rofi -show drun")"#), "{ron}");
|
||||
assert!(ron.contains(r#"(modifiers: [Super], key: "q"): Close"#), "{ron}");
|
||||
assert!(
|
||||
ron.contains(r#"(modifiers: [Super], key: "d"): Spawn("rofi -show drun")"#),
|
||||
"{ron}"
|
||||
);
|
||||
assert!(
|
||||
ron.contains(r#"(modifiers: [Super], key: "q"): Close"#),
|
||||
"{ron}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -449,7 +469,11 @@ mod tests {
|
||||
let d = errors("bind = SUPER, D, exec, rofi -show drun\nbind = SUPER, D, killactive\n");
|
||||
assert_eq!(d.len(), 1);
|
||||
assert!(d[0].message.contains("already bound"), "{}", d[0].message);
|
||||
assert!(d[0].help.as_ref().unwrap().contains("line 1"), "{:?}", d[0].help);
|
||||
assert!(
|
||||
d[0].help.as_ref().unwrap().contains("line 1"),
|
||||
"{:?}",
|
||||
d[0].help
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -577,7 +601,11 @@ mod tests {
|
||||
#[test]
|
||||
fn invalid_theme_mode_is_rejected() {
|
||||
let d = errors("theme {\n mode = purple\n}\n");
|
||||
assert!(d[0].message.contains("`dark` or `light`"), "{}", d[0].message);
|
||||
assert!(
|
||||
d[0].message.contains("`dark` or `light`"),
|
||||
"{}",
|
||||
d[0].message
|
||||
);
|
||||
}
|
||||
|
||||
/// `#` always begins a comment, so a bare hex colour is stripped before it
|
||||
@@ -600,12 +628,26 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_multibyte_character_in_a_colour_is_an_error_not_a_panic() {
|
||||
// `hex.len()` and the slicing that follows it are both in bytes, so
|
||||
// "€abc" is six bytes and would have been sliced mid-character.
|
||||
for raw in ["rgb(€abc)", "rgba(ff€€ff00)", "rgb(αβγ)"] {
|
||||
let d = errors(&format!("theme {{\n accent = {raw}\n}}\n"));
|
||||
assert_eq!(d.len(), 1, "{raw}");
|
||||
assert!(d[0].message.contains("colour"), "{}", d[0].message);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_key_suggests_the_near_miss() {
|
||||
let d = errors("general {\n gaps_inn = 8\n}\n");
|
||||
assert_eq!(d.len(), 1);
|
||||
assert!(d[0].message.contains("unknown key"), "{}", d[0].message);
|
||||
assert_eq!(d[0].help.as_deref(), Some("did you mean `general.gaps_in`?"));
|
||||
assert_eq!(
|
||||
d[0].help.as_deref(),
|
||||
Some("did you mean `general.gaps_in`?")
|
||||
);
|
||||
assert_eq!(d[0].span.line, 2);
|
||||
}
|
||||
|
||||
@@ -613,13 +655,21 @@ mod tests {
|
||||
fn type_errors_are_reported_against_the_value() {
|
||||
let d = errors("general {\n gaps_in = purple\n}\n");
|
||||
assert_eq!(d.len(), 1);
|
||||
assert!(d[0].message.contains("non-negative integer"), "{}", d[0].message);
|
||||
assert!(
|
||||
d[0].message.contains("non-negative integer"),
|
||||
"{}",
|
||||
d[0].message
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn out_of_range_values_are_rejected() {
|
||||
let d = errors("general {\n gaps_in = 9999\n}\n");
|
||||
assert!(d[0].message.contains("outside the allowed range"), "{}", d[0].message);
|
||||
assert!(
|
||||
d[0].message.contains("outside the allowed range"),
|
||||
"{}",
|
||||
d[0].message
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
+85
-25
@@ -79,16 +79,20 @@ impl fmt::Display for CompileError {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
CompileError::Read { path, error } => {
|
||||
write!(f, "error: cannot read {}: {error}\n", path.display())
|
||||
writeln!(f, "error: cannot read {}: {error}", path.display())
|
||||
}
|
||||
CompileError::Cycle { path } => {
|
||||
write!(
|
||||
writeln!(
|
||||
f,
|
||||
"error: `source` cycle detected while expanding {}\n",
|
||||
"error: `source` cycle detected while expanding {}",
|
||||
path.display()
|
||||
)
|
||||
}
|
||||
CompileError::Parse { path, source, error } => {
|
||||
CompileError::Parse {
|
||||
path,
|
||||
source,
|
||||
error,
|
||||
} => {
|
||||
write!(
|
||||
f,
|
||||
"in {}:\n{}",
|
||||
@@ -96,10 +100,18 @@ impl fmt::Display for CompileError {
|
||||
render_diagnostic(source, error.span, &error.message, None)
|
||||
)
|
||||
}
|
||||
CompileError::Resolve { source, diagnostics } => {
|
||||
CompileError::Resolve {
|
||||
source,
|
||||
diagnostics,
|
||||
} => {
|
||||
let mut out = String::new();
|
||||
for d in diagnostics {
|
||||
out.push_str(&render_diagnostic(source, d.span, &d.message, d.help.as_deref()));
|
||||
out.push_str(&render_diagnostic(
|
||||
source,
|
||||
d.span,
|
||||
&d.message,
|
||||
d.help.as_deref(),
|
||||
));
|
||||
out.push('\n');
|
||||
}
|
||||
out.push_str(&format!(
|
||||
@@ -200,7 +212,7 @@ fn merge_text(
|
||||
|
||||
// Splicing changes line counts, so process bottom-up: replacing a later
|
||||
// line first leaves every earlier line number still valid.
|
||||
targets.sort_by(|a, b| b.0.cmp(&a.0));
|
||||
targets.sort_by_key(|t| std::cmp::Reverse(t.0));
|
||||
|
||||
ancestors.push(key);
|
||||
let mut lines: Vec<String> = raw.lines().map(String::from).collect();
|
||||
@@ -236,7 +248,9 @@ fn collect_sources(items: &[Item], out: &mut Vec<(usize, String)>) {
|
||||
/// it is checked out.
|
||||
fn resolve_source_path(containing_file: &Path, raw: &str) -> PathBuf {
|
||||
let expanded = if raw == "~" {
|
||||
std::env::var_os("HOME").map(PathBuf::from).unwrap_or_else(|| PathBuf::from(raw))
|
||||
std::env::var_os("HOME")
|
||||
.map(PathBuf::from)
|
||||
.unwrap_or_else(|| PathBuf::from(raw))
|
||||
} else if let Some(rest) = raw.strip_prefix("~/") {
|
||||
match std::env::var_os("HOME") {
|
||||
Some(home) => PathBuf::from(home).join(rest),
|
||||
@@ -280,7 +294,11 @@ fn collect_batch<T>(rx: &mpsc::Receiver<T>, window: Duration) -> Option<Vec<T>>
|
||||
/// Best-effort: a `watch`/`unwatch` failure (e.g. a sourced file that does
|
||||
/// not exist yet) is not fatal — the next successful compile will retry with
|
||||
/// whatever the config asks for at that point.
|
||||
fn sync_watches(watcher: &mut RecommendedWatcher, current: &mut HashSet<PathBuf>, wanted: &[PathBuf]) {
|
||||
fn sync_watches(
|
||||
watcher: &mut RecommendedWatcher,
|
||||
current: &mut HashSet<PathBuf>,
|
||||
wanted: &[PathBuf],
|
||||
) {
|
||||
let wanted: HashSet<PathBuf> = wanted.iter().cloned().collect();
|
||||
|
||||
for stale in current.difference(&wanted) {
|
||||
@@ -340,7 +358,11 @@ pub fn watch(config: &Path, emitter: &Emitter) -> Result<(), WatchError> {
|
||||
}
|
||||
Err(e) => {
|
||||
eprintln!("{e}");
|
||||
sync_watches(&mut watcher, &mut watched, std::slice::from_ref(&config.to_path_buf()));
|
||||
sync_watches(
|
||||
&mut watcher,
|
||||
&mut watched,
|
||||
std::slice::from_ref(&config.to_path_buf()),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -394,7 +416,11 @@ mod tests {
|
||||
fn compile_with_no_source_directives_watches_just_the_config() {
|
||||
let conf_dir = TempDir::new().unwrap();
|
||||
let root_dir = TempDir::new().unwrap();
|
||||
let config = write(conf_dir.path(), "cosmic.conf", "general {\n autotile = true\n}\n");
|
||||
let config = write(
|
||||
conf_dir.path(),
|
||||
"cosmic.conf",
|
||||
"general {\n autotile = true\n}\n",
|
||||
);
|
||||
|
||||
let compiled = compile(&config, &Emitter::with_root(root_dir.path())).unwrap();
|
||||
|
||||
@@ -406,7 +432,11 @@ mod tests {
|
||||
fn compile_follows_a_source_directive_and_lists_it_as_a_watch_target() {
|
||||
let conf_dir = TempDir::new().unwrap();
|
||||
let root_dir = TempDir::new().unwrap();
|
||||
let included = write(conf_dir.path(), "extra.conf", "general {\n autotile = true\n}\n");
|
||||
let included = write(
|
||||
conf_dir.path(),
|
||||
"extra.conf",
|
||||
"general {\n autotile = true\n}\n",
|
||||
);
|
||||
let config = write(conf_dir.path(), "cosmic.conf", "source = extra.conf\n");
|
||||
|
||||
let compiled = compile(&config, &Emitter::with_root(root_dir.path())).unwrap();
|
||||
@@ -431,7 +461,10 @@ mod tests {
|
||||
let planned = compile(&config, &Emitter::with_root(root_dir.path()))
|
||||
.unwrap()
|
||||
.planned;
|
||||
let gaps = planned.iter().find(|p| p.path.ends_with("gaps")).expect("gaps planned");
|
||||
let gaps = planned
|
||||
.iter()
|
||||
.find(|p| p.path.ends_with("gaps"))
|
||||
.expect("gaps planned");
|
||||
assert_eq!(gaps.contents, "(10, 5)");
|
||||
}
|
||||
|
||||
@@ -453,7 +486,11 @@ mod tests {
|
||||
#[test]
|
||||
fn compile_expands_tilde_against_home() {
|
||||
let home = TempDir::new().unwrap();
|
||||
write(home.path(), "shared.conf", "general {\n autotile = true\n}\n");
|
||||
write(
|
||||
home.path(),
|
||||
"shared.conf",
|
||||
"general {\n autotile = true\n}\n",
|
||||
);
|
||||
let conf_dir = TempDir::new().unwrap();
|
||||
let config = write(conf_dir.path(), "cosmic.conf", "source = ~/shared.conf\n");
|
||||
|
||||
@@ -473,10 +510,15 @@ mod tests {
|
||||
fn compile_follows_a_chain_of_nested_sources() {
|
||||
let conf_dir = TempDir::new().unwrap();
|
||||
write(conf_dir.path(), "c.conf", "autotile = true\n");
|
||||
write(conf_dir.path(), "b.conf", "general {\n source = c.conf\n}\n");
|
||||
write(
|
||||
conf_dir.path(),
|
||||
"b.conf",
|
||||
"general {\n source = c.conf\n}\n",
|
||||
);
|
||||
let config = write(conf_dir.path(), "a.conf", "source = b.conf\n");
|
||||
|
||||
let compiled = compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap();
|
||||
let compiled =
|
||||
compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap();
|
||||
assert_eq!(compiled.sources.len(), 3);
|
||||
assert_eq!(compiled.planned.len(), 1);
|
||||
}
|
||||
@@ -496,9 +538,14 @@ mod tests {
|
||||
#[test]
|
||||
fn compile_reports_a_missing_source_file_without_panicking() {
|
||||
let conf_dir = TempDir::new().unwrap();
|
||||
let config = write(conf_dir.path(), "cosmic.conf", "source = does-not-exist.conf\n");
|
||||
let config = write(
|
||||
conf_dir.path(),
|
||||
"cosmic.conf",
|
||||
"source = does-not-exist.conf\n",
|
||||
);
|
||||
|
||||
let err = compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap_err();
|
||||
let err =
|
||||
compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap_err();
|
||||
assert!(matches!(err, CompileError::Read { .. }), "{err}");
|
||||
}
|
||||
|
||||
@@ -508,9 +555,12 @@ mod tests {
|
||||
write(conf_dir.path(), "broken.conf", "this is not valid\n");
|
||||
let config = write(conf_dir.path(), "cosmic.conf", "source = broken.conf\n");
|
||||
|
||||
let err = compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap_err();
|
||||
let err =
|
||||
compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap_err();
|
||||
match err {
|
||||
CompileError::Parse { path, .. } => assert_eq!(path, conf_dir.path().join("broken.conf")),
|
||||
CompileError::Parse { path, .. } => {
|
||||
assert_eq!(path, conf_dir.path().join("broken.conf"))
|
||||
}
|
||||
other => panic!("expected Parse, got {other}"),
|
||||
}
|
||||
}
|
||||
@@ -528,7 +578,8 @@ mod tests {
|
||||
"general {\n source = bad.conf\n}\n",
|
||||
);
|
||||
|
||||
let err = compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap_err();
|
||||
let err =
|
||||
compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap_err();
|
||||
match err {
|
||||
CompileError::Resolve { diagnostics, .. } => {
|
||||
assert_eq!(diagnostics[0].span.line, 2);
|
||||
@@ -540,9 +591,14 @@ mod tests {
|
||||
#[test]
|
||||
fn compile_surfaces_resolve_diagnostics_for_an_unknown_key() {
|
||||
let conf_dir = TempDir::new().unwrap();
|
||||
let config = write(conf_dir.path(), "cosmic.conf", "general {\n gaps_inn = 8\n}\n");
|
||||
let config = write(
|
||||
conf_dir.path(),
|
||||
"cosmic.conf",
|
||||
"general {\n gaps_inn = 8\n}\n",
|
||||
);
|
||||
|
||||
let err = compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap_err();
|
||||
let err =
|
||||
compile(&config, &Emitter::with_root(TempDir::new().unwrap().path())).unwrap_err();
|
||||
assert!(matches!(err, CompileError::Resolve { .. }), "{err}");
|
||||
assert!(err.to_string().contains("unknown key"), "{err}");
|
||||
}
|
||||
@@ -552,7 +608,11 @@ mod tests {
|
||||
#[test]
|
||||
fn compile_does_not_write_to_the_config_root() {
|
||||
let conf_dir = TempDir::new().unwrap();
|
||||
let config = write(conf_dir.path(), "cosmic.conf", "general {\n autotile = true\n}\n");
|
||||
let config = write(
|
||||
conf_dir.path(),
|
||||
"cosmic.conf",
|
||||
"general {\n autotile = true\n}\n",
|
||||
);
|
||||
let root_dir = TempDir::new().unwrap();
|
||||
|
||||
let _ = compile(&config, &Emitter::with_root(root_dir.path())).unwrap();
|
||||
@@ -655,7 +715,7 @@ mod tests {
|
||||
|
||||
// Dropping `b` from the wanted set must unwatch it, not just stop
|
||||
// tracking it, or the watch set would only ever grow.
|
||||
sync_watches(&mut watcher, &mut current, &[a.clone()]);
|
||||
sync_watches(&mut watcher, &mut current, std::slice::from_ref(&a));
|
||||
assert_eq!(current, HashSet::from([a]));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user