always sort assets by filename (Windows/Linux difference) (#2236)

* sort page.assets by filename

Uses .to_str() to sort files and subfolders.

The .unwrap() may need work or be replaced by unwrap_or_default(). Given
earlier checks in the function it should work however.

* add tests for assets sorting

* fix rustfmt

* use existing loop instead of windows

* also check the non-recursive test

* use .zip() and add assert msg
This commit is contained in:
William Ouwehand 2023-07-10 23:33:16 +02:00 committed by Vincent Prouillet
parent 2532198acb
commit c18a0c8031

View File

@ -25,6 +25,7 @@ pub fn has_anchor(headings: &[Heading], anchor: &str) -> bool {
/// If `recursive` is set to `true`, it will add all subdirectories assets as well. This should /// If `recursive` is set to `true`, it will add all subdirectories assets as well. This should
/// only be set when finding page assets currently. /// only be set when finding page assets currently.
/// TODO: remove this flag once sections with assets behave the same as pages with assets /// TODO: remove this flag once sections with assets behave the same as pages with assets
/// The returned vector with assets is sorted in case-sensitive order (using `to_ascii_lowercase()`)
pub fn find_related_assets(path: &Path, config: &Config, recursive: bool) -> Vec<PathBuf> { pub fn find_related_assets(path: &Path, config: &Config, recursive: bool) -> Vec<PathBuf> {
let mut assets = vec![]; let mut assets = vec![];
@ -50,6 +51,10 @@ pub fn find_related_assets(path: &Path, config: &Config, recursive: bool) -> Vec
assets.retain(|p| !globset.is_match(p)); assets.retain(|p| !globset.is_match(p));
} }
assets.sort_by(|a, b| {
a.to_str().unwrap().to_ascii_lowercase().cmp(&b.to_str().unwrap().to_ascii_lowercase())
});
assets assets
} }
@ -82,13 +87,31 @@ mod tests {
create_dir(path.join("subdir")).expect("create subdir temp dir"); create_dir(path.join("subdir")).expect("create subdir temp dir");
File::create(path.join("subdir").join("index.md")).unwrap(); File::create(path.join("subdir").join("index.md")).unwrap();
File::create(path.join("subdir").join("example.js")).unwrap(); File::create(path.join("subdir").join("example.js")).unwrap();
File::create(path.join("FFF.txt")).unwrap();
File::create(path.join("GRAPH.txt")).unwrap();
File::create(path.join("subdir").join("GGG.txt")).unwrap();
let assets = find_related_assets(path, &Config::default(), true); let assets = find_related_assets(path, &Config::default(), true);
assert_eq!(assets.len(), 4); assert_eq!(assets.len(), 7);
assert_eq!(assets.iter().filter(|p| p.extension().unwrap_or_default() != "md").count(), 4); assert_eq!(assets.iter().filter(|p| p.extension().unwrap_or_default() != "md").count(), 7);
for asset in ["example.js", "graph.jpg", "fail.png", "subdir/example.js"] { // Use case-insensitive ordering for testassets
assert!(assets.iter().any(|p| p.strip_prefix(path).unwrap() == Path::new(asset))) let testassets = [
"example.js",
"fail.png",
"FFF.txt",
"graph.jpg",
"GRAPH.txt",
"subdir/example.js",
"subdir/GGG.txt",
];
for (asset, testasset) in assets.iter().zip(testassets.iter()) {
assert!(
asset.strip_prefix(path).unwrap() == Path::new(testasset),
"Mismatch between asset {} and testasset {}",
asset.to_str().unwrap(),
testasset
);
} }
} }
@ -104,12 +127,23 @@ mod tests {
create_dir(path.join("subdir")).expect("create subdir temp dir"); create_dir(path.join("subdir")).expect("create subdir temp dir");
File::create(path.join("subdir").join("index.md")).unwrap(); File::create(path.join("subdir").join("index.md")).unwrap();
File::create(path.join("subdir").join("example.js")).unwrap(); File::create(path.join("subdir").join("example.js")).unwrap();
let assets = find_related_assets(path, &Config::default(), false); File::create(path.join("FFF.txt")).unwrap();
assert_eq!(assets.len(), 3); File::create(path.join("GRAPH.txt")).unwrap();
assert_eq!(assets.iter().filter(|p| p.extension().unwrap_or_default() != "md").count(), 3); File::create(path.join("subdir").join("GGG.txt")).unwrap();
for asset in ["example.js", "graph.jpg", "fail.png"] { let assets = find_related_assets(path, &Config::default(), false);
assert!(assets.iter().any(|p| p.strip_prefix(path).unwrap() == Path::new(asset))) assert_eq!(assets.len(), 5);
assert_eq!(assets.iter().filter(|p| p.extension().unwrap_or_default() != "md").count(), 5);
// Use case-insensitive ordering for testassets
let testassets = ["example.js", "fail.png", "FFF.txt", "graph.jpg", "GRAPH.txt"];
for (asset, testasset) in assets.iter().zip(testassets.iter()) {
assert!(
asset.strip_prefix(path).unwrap() == Path::new(testasset),
"Mismatch between asset {} and testasset {}",
asset.to_str().unwrap(),
testasset
);
} }
} }
#[test] #[test]