pauljd/vectorcraft
1
0
Fork 0
mirror of https://github.com/storytold/vectorcraft.git synced 2026-11-07 12:13:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #5928 from adikshan11/fix/library-untitled-format-copies

Browse Source 
Don't copy a library on every save when its format keeps no title
This commit is contained in:
BFlat
2026-11-07 09:02:27 +08:00
committed by GitHub
parent d074756192 4e11389bf0
commit c14ec164a2
2 changed files with 189 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd/swatchlib.rs
+61 -10
View File
@@ -6,7 +6,7 @@
//! pattern) into the document; `swatch.library.save` writes the document's ([`palette_io`]).
//! `swatch.library.copyToUser` copies a loaded library into the user library folder.
use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
use std::sync::Arc;
use serde_json::{Value, json};
@@ -104,6 +104,19 @@ pub struct Libraries<L = SwatchLibrary> {
/// Loaded libraries copied into the user library folder ([`Libraries::copy_to_user`]): the
/// loaded id and the User Defined id [`Libraries::get`] finds for it.
copied: Vec<(String, String)>,
/// The name each User Defined file was saved under ([`Libraries::write`]), lower case id first,
/// with a hash of the bytes written. A format that keeps no title (`.ase`) reads its name from
/// the file name instead. [`Libraries::rescan`] drops an entry whose file no longer holds those
/// bytes, and keeps one whose file is missing so a file put back can be checked.
saved_as: HashMap<String, SavedAs>,
}
/// A User Defined file this session wrote: the library's name, and [`vectorcraft_doc::links::hash_bytes`]
/// of what was written, so a file replaced since then is no longer that library's.
#[derive(Clone, Debug)]
struct SavedAs {
name: String,
hash: String,
}
#[derive(Clone, Debug)]
@@ -152,20 +165,30 @@ impl<L: LibraryFile> Libraries<L> {
/// Set the user library folder and list its libraries.
pub fn set_user_dir(&mut self, dir: Option<String>) {
self.user_dir = dir;
self.saved_as.clear();
self.rescan();
}
/// Re-read the library files of the user library folder (unreadable ones are skipped). The
/// folder is the app's own: automation roots don't apply to it ([`crate::file_access`]). Each
/// file is read inside [`crate::guard::catch_panic`], and a file whose reading panics is
/// skipped too.
/// skipped too. A file this session saved, whose bytes are no longer the ones written, is
/// forgotten ([`Libraries::write`]); a missing file is left remembered until it is back.
pub fn rescan(&mut self) {
self.extra.retain(|e| e.info.category != "user");
let Some(dir) = self.user_dir.clone() else { return };
for path in library_files(&dir, L::EXTS) {
let file = file_name(&path);
let read = || crate::file_access::unconfined(|| read_file(&path)).ok().and_then(|b| L::read(&b, stem(&file)).ok());
let Some(lib) = crate::guard::catch_panic(read).ok().flatten() else { continue };
let read = || crate::file_access::unconfined(|| read_file(&path)).ok();
let Some(bytes) = crate::guard::catch_panic(read).ok().flatten() else { continue };
let id = format!("user/{file}").to_lowercase();
// Compared before parsing, so a file that no longer reads as a library is forgotten too.
let hash = vectorcraft_doc::links::hash_bytes(&bytes);
if self.saved_as.get(&id).is_some_and(|saved| saved.hash != hash) {
self.saved_as.remove(&id);
}
let parsed = || L::read(&bytes, stem(&file)).ok();
let Some(lib) = crate::guard::catch_panic(parsed).ok().flatten() else { continue };
let info = LibraryInfo { id: format!("user/{file}"), name: library_tab_name(&file, lib.name()), category: "user" };
self.extra.push(Extra { info, path: Some(path), lib: Arc::new(lib) });
}
@@ -214,17 +237,36 @@ impl<L: LibraryFile> Libraries<L> {
let mut file = library_file_name(name, ext);
// `COM¹` is saved as `COM¹-`, which can be another library's own name, and a library put
// under a numbered name holds a file another name can ask for: such a file is kept.
// `saved_as` is that memory for this run, and only while the file still holds what was
// written. A file that could not be read has no name to trust: one a device name could
// make is kept too. Any other unreadable file is replaced, as it was before.
self.rescan();
let device = crate::file_access::is_device_name;
let id = format!("user/{file}").to_lowercase();
let fallback = |other: &str| library_file_name(other, ext).to_lowercase() != file.to_lowercase();
if self
.extra
.iter()
.any(|e| e.info.id.to_lowercase() == id && e.lib.name() != name && (device(name) || device(e.lib.name()) || fallback(e.lib.name())))
{
// A format with no title reads its name from the file name, so a file a device name could
// have made has no known owner until it is saved here.
let untitled = L::read(bytes, "\0").is_ok_and(|lib| lib.name() == "\0");
let owner = |e: &Extra<L>| match self.saved_as.get(&e.info.id.to_lowercase()) {
Some(saved) => Some(saved.name.clone()),
None if untitled && device_file_name(e.lib.name()) => None,
None => Some(e.lib.name().to_string()),
};
let file_key = file.to_lowercase();
let on_disk = library_files(&dir, L::EXTS).iter().any(|p| file_name(p).to_lowercase() == file_key);
let listed = self.extra.iter().any(|e| e.info.category == "user" && e.info.id.to_lowercase() == id);
let unread = on_disk && !listed && (device(name) || device_file_name(stem(&file)));
let held = unread
|| self.extra.iter().any(|e| {
e.info.id.to_lowercase() == id
&& owner(e).is_none_or(|other| other != name && (device(name) || device(&other) || fallback(&other)))
});
if held {
// Saved again, it goes back to the file it was given the first time.
let own = self.extra.iter().find(|e| e.info.category == "user" && e.lib.name() == name && extension(&e.info.id) == extension(&file));
let own = self
.extra
.iter()
.find(|e| e.info.category == "user" && owner(e).as_deref() == Some(name) && extension(&e.info.id) == extension(&file));
file = match own.and_then(|e| e.info.id.strip_prefix("user/")) {
Some(own) => own.to_string(),
None => {
@@ -236,6 +278,8 @@ impl<L: LibraryFile> Libraries<L> {
let path = std::path::Path::new(&dir).join(&file).to_string_lossy().to_string();
// The app's own folder, under a name made safe: no root applies.
crate::file_access::unconfined(|| create_dir(&dir).and_then(|()| write_file(&path, bytes)))?;
self.saved_as
.insert(format!("user/{file}").to_lowercase(), SavedAs { name: name.to_string(), hash: vectorcraft_doc::links::hash_bytes(bytes) });
self.rescan();
out["path"] = json!(path);
out["library"] = json!(format!("user/{file}"));
@@ -373,6 +417,13 @@ pub fn library_file_name(name: &str, ext: &str) -> String {
}
}
/// Is `stem` a file name [`library_file_name`] gives a device name (`COM¹-`, `COM¹-.x`), or one of
/// those with the number [`free_name`] adds (`COM¹- 2`)?
fn device_file_name(stem: &str) -> bool {
let base = stem.rsplit_once(' ').filter(|(_, n)| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())).map_or(stem, |(base, _)| base);
base.split('.').next().and_then(|first| first.strip_suffix('-')).is_some_and(crate::file_access::is_device_name)
}
/// The library files with extensions `exts` in folder `dir`, sorted by name.
#[cfg(not(target_arch = "wasm32"))]
fn library_files(dir: &str, exts: &[&str]) -> Vec<String> {
crates/engine/src/tests/swatchlib.rs
+128
View File
@@ -792,6 +792,134 @@ fn a_library_named_like_a_device_keeps_the_library_with_its_file_name() {
}
}
/// `.ase` keeps no title, so a library's name comes back from its file name. Whichever is saved
/// first, `COM¹` and `COM¹-` keep a file each, and saving either again replaces only itself. After
/// a restart neither can be told from the other, so each gets a new file and replaces neither.
#[test]
fn device_named_libraries_without_a_title_keep_their_own_files() {
for (first, second) in [("COM¹", "COM¹-"), ("COM¹-", "COM¹")] {
let dir = temp_dir("device-ase");
let mut s = session();
s.swatch_libraries.set_user_dir(Some(dir.to_string_lossy().to_string()));
let save = |s: &mut Session, name: &str| {
run(s, "swatch.library.save", json!({"user": true, "name": name, "format": "ase"}))["library"].as_str().unwrap().to_string()
};
let a = save(&mut s, first);
assert_eq!(save(&mut s, first), a, "saving {first} again replaces {first}");
let b = save(&mut s, second);
assert_ne!(a, b);
assert_eq!(save(&mut s, first), a, "saving {first} again replaces {first}");
assert_eq!(save(&mut s, second), b, "saving {second} again replaces {second}");
assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 2);
// The next time the app starts.
s.swatch_libraries.set_user_dir(Some(dir.to_string_lossy().to_string()));
for (name, count) in [(first, 3), (second, 4)] {
let again = save(&mut s, name);
assert!(again != a && again != b, "{name}: {again}");
assert_eq!(save(&mut s, name), again, "saving {name} again replaces {name}");
assert_eq!(std::fs::read_dir(&dir).unwrap().count(), count);
}
// The numbered file is as unknown as the first.
s.swatch_libraries.set_user_dir(Some(dir.to_string_lossy().to_string()));
let numbered = b.strip_prefix("user/").unwrap().strip_suffix(".ase").unwrap().to_string();
assert_ne!(save(&mut s, &numbered), b);
assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 5);
// Any other name goes on replacing its own file after a restart.
let other = save(&mut s, "Greys");
s.swatch_libraries.set_user_dir(Some(dir.to_string_lossy().to_string()));
assert_eq!(save(&mut s, "Greys"), other);
let _ = std::fs::remove_dir_all(&dir);
}
}
/// A file this session saved for `COM¹`, then replaced, is not written over. The next save goes to
/// a new file and stays there. That holds for `.ase` and `.gpl`, for bytes that still read as a
/// library and for bytes that don't, including the file the library moved to. A file removed and
/// put back is still its library's when the bytes are the ones written, and is kept when they are
/// not. A name that is not a device name still replaces a file that cannot be read.
#[test]
fn a_replaced_device_library_file_is_kept() {
for format in ["ase", "gpl"] {
let other: Vec<u8> = if format == "ase" {
vectorcraft_testkit::ase::ase(&[vectorcraft_testkit::ase::Block::Color("Only", b"RGB ", &[1.0, 0.0, 0.0], 2)])
} else {
b"GIMP Palette\nName: Other\n0 0 0 Black\n".to_vec()
};
let save = |s: &mut Session| run(s, "swatch.library.save", json!({"user": true, "name": "COM¹", "format": format}));
let open = |name: &str| {
let dir = temp_dir(name);
let mut s = session();
s.swatch_libraries.set_user_dir(Some(dir.to_string_lossy().to_string()));
(dir, s)
};
// Replaced with another library.
let (dir, mut s) = open(&format!("replaced-lib-{format}"));
let first = save(&mut s);
let first_id = first["library"].as_str().unwrap().to_string();
let first_path = first["path"].as_str().unwrap().to_string();
assert_ne!(std::fs::read(&first_path).unwrap(), other);
std::fs::write(&first_path, &other).unwrap();
let moved = save(&mut s);
let moved_id = moved["library"].as_str().unwrap().to_string();
assert_ne!(moved_id, first_id);
assert_eq!(std::fs::read(&first_path).unwrap(), other, "{format}: the replaced file is left as it is");
assert_eq!(save(&mut s)["library"].as_str(), Some(moved_id.as_str()), "{format}: saving again replaces the new file");
let _ = std::fs::remove_dir_all(&dir);
// Replaced with bytes that are not a library, first the file it had, then the one it moved to.
let (dir, mut s) = open(&format!("replaced-unread-{format}"));
let first = save(&mut s);
let first_path = first["path"].as_str().unwrap().to_string();
std::fs::write(&first_path, b"not a library").unwrap();
let moved = save(&mut s);
let moved_id = moved["library"].as_str().unwrap().to_string();
let first_id = first["library"].as_str().unwrap();
assert_ne!(moved_id, first_id);
assert_eq!(std::fs::read(&first_path).unwrap(), b"not a library");
assert_eq!(save(&mut s)["library"].as_str(), Some(moved_id.as_str()), "{format}: saving again stays on the new file");
let moved_path = moved["path"].as_str().unwrap().to_string();
std::fs::write(&moved_path, b"still not").unwrap();
let again = save(&mut s);
let again_id = again["library"].as_str().unwrap().to_string();
assert!(again_id != moved_id && again_id != first_id);
assert_eq!(std::fs::read(&moved_path).unwrap(), b"still not");
assert_eq!(save(&mut s)["library"].as_str(), Some(again_id.as_str()));
let _ = std::fs::remove_dir_all(&dir);
// Removed, the folder listed again, then put back: the hash is checked when it reappears.
let (dir, mut s) = open(&format!("replaced-reappear-{format}"));
let first = save(&mut s);
let first_id = first["library"].as_str().unwrap().to_string();
let first_path = first["path"].as_str().unwrap().to_string();
let written = std::fs::read(&first_path).unwrap();
std::fs::remove_file(&first_path).unwrap();
run(&mut s, "swatch.library.list", json!({}));
std::fs::write(&first_path, &written).unwrap();
assert_eq!(save(&mut s)["library"].as_str(), Some(first_id.as_str()), "{format}: the same bytes are still this library");
std::fs::remove_file(&first_path).unwrap();
run(&mut s, "swatch.library.list", json!({}));
std::fs::write(&first_path, &other).unwrap();
let moved = save(&mut s);
let moved_id = moved["library"].as_str().unwrap().to_string();
assert_ne!(moved_id, first_id);
assert_eq!(std::fs::read(&first_path).unwrap(), other, "{format}: different bytes put back are kept");
assert_eq!(save(&mut s)["library"].as_str(), Some(moved_id.as_str()));
let _ = std::fs::remove_dir_all(&dir);
}
let dir = temp_dir("replaced-ordinary");
let mut s = session();
s.swatch_libraries.set_user_dir(Some(dir.to_string_lossy().to_string()));
let first = run(&mut s, "swatch.library.save", json!({"user": true, "name": "Greys", "format": "gpl"}));
let path = first["path"].as_str().unwrap().to_string();
std::fs::write(&path, b"not a library").unwrap();
let again = run(&mut s, "swatch.library.save", json!({"user": true, "name": "Greys", "format": "gpl"}));
assert_eq!(again["library"], first["library"], "a name that is not a device name replaces its own unreadable file");
assert!(std::fs::read(&path).unwrap().starts_with(b"GIMP Palette"));
let _ = std::fs::remove_dir_all(&dir);
}
/// #3602: a library titled with a Windows device name is saved under a name that is not one.
#[test]
fn a_library_file_name_is_never_a_windows_device_name() {