From c152addc3b974c1fdae3f4e0d4306fc10e886faa Mon Sep 17 00:00:00 2001 From: Yongwoo Lee Date: Thu, 17 Oct 2024 00:03:21 +0900 Subject: [PATCH 1/7] feat: sort the order of bulk-renamed files --- yazi-core/src/manager/commands/bulk_rename.rs | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/yazi-core/src/manager/commands/bulk_rename.rs b/yazi-core/src/manager/commands/bulk_rename.rs index 88bf79d2c..7e0545387 100644 --- a/yazi-core/src/manager/commands/bulk_rename.rs +++ b/yazi-core/src/manager/commands/bulk_rename.rs @@ -53,7 +53,7 @@ impl Manager { return Ok(()); } - let todo: Vec<_> = old.into_iter().zip(new).filter(|(o, n)| o != n).collect(); + let todo = Self::sort(old, new); if todo.is_empty() { return Ok(()); } @@ -117,4 +117,44 @@ impl Manager { stdin().read_exact(&mut [0]).await?; Ok(()) } + + fn sort(old: Vec, new: Vec) -> Vec<(PathBuf, PathBuf)> { + let mut income_map: HashMap = + old.iter().map(|path| (path.clone(), false)).collect(); + let mut todos: HashMap = old + .into_iter() + .zip(new) + .map(|(old, new)| { + if let Some(has_income) = income_map.get_mut(&new) { + *has_income = true; + } + (old, new) + }) + .collect(); + + let mut sorted = vec![]; + while !todos.is_empty() { + let mut has_no_incomes = vec![]; + income_map.iter().for_each(|(old, has_income)| { + if !has_income { + has_no_incomes.push(old.clone()) + } + }); + + if has_no_incomes.is_empty() { + todo!("Consider cycle") + } + + for old in has_no_incomes { + income_map.remove(&old); + let Some(new) = todos.remove(&old) else { unreachable!("") }; + if let Some(has_income) = income_map.get_mut(&new) { + *has_income = false; + } + sorted.push((old, new)); + } + } + sorted.reverse(); + sorted + } } From 85597617d50cb96f20ffa7065d31b1d0890ec1de Mon Sep 17 00:00:00 2001 From: Yongwoo Lee Date: Fri, 18 Oct 2024 19:28:02 +0900 Subject: [PATCH 2/7] feat: add unittest for sort --- yazi-core/src/manager/commands/bulk_rename.rs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/yazi-core/src/manager/commands/bulk_rename.rs b/yazi-core/src/manager/commands/bulk_rename.rs index 7e0545387..19e9126b9 100644 --- a/yazi-core/src/manager/commands/bulk_rename.rs +++ b/yazi-core/src/manager/commands/bulk_rename.rs @@ -158,3 +158,23 @@ impl Manager { sorted } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sort() { + let old = vec!["1", "2", "3", "4", "5"].into_iter().map(PathBuf::from).collect(); + let new = vec!["2", "3", "4", "5", "6"].into_iter().map(PathBuf::from).collect(); + + let mut sorted = Manager::sort(old, new).into_iter(); + + assert_eq!(sorted.next(), Some((PathBuf::from("5"), PathBuf::from("6")))); + assert_eq!(sorted.next(), Some((PathBuf::from("4"), PathBuf::from("5")))); + assert_eq!(sorted.next(), Some((PathBuf::from("3"), PathBuf::from("4")))); + assert_eq!(sorted.next(), Some((PathBuf::from("2"), PathBuf::from("3")))); + assert_eq!(sorted.next(), Some((PathBuf::from("1"), PathBuf::from("2")))); + assert_eq!(sorted.next(), None); + } +} From b8341e23f98ead4ca7fee239ddf42eb4efec82d8 Mon Sep 17 00:00:00 2001 From: sxyazi Date: Sat, 19 Oct 2024 22:30:23 +0800 Subject: [PATCH 3/7] Add more tests --- yazi-core/src/manager/commands/bulk_rename.rs | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/yazi-core/src/manager/commands/bulk_rename.rs b/yazi-core/src/manager/commands/bulk_rename.rs index 19e9126b9..c3b946609 100644 --- a/yazi-core/src/manager/commands/bulk_rename.rs +++ b/yazi-core/src/manager/commands/bulk_rename.rs @@ -165,16 +165,32 @@ mod tests { #[test] fn test_sort() { - let old = vec!["1", "2", "3", "4", "5"].into_iter().map(PathBuf::from).collect(); - let new = vec!["2", "3", "4", "5", "6"].into_iter().map(PathBuf::from).collect(); - - let mut sorted = Manager::sort(old, new).into_iter(); + fn cmp(input: &[(&str, &str)], expected: &[(&str, &str)]) { + let sorted = Manager::sort( + input.iter().map(|&(o, _)| o.into()).collect(), + input.iter().map(|&(_, n)| n.into()).collect(), + ); + let sorted: Vec<_> = + sorted.iter().map(|(o, n)| (o.to_str().unwrap(), n.to_str().unwrap())).collect(); + assert_eq!(sorted, expected); + } - assert_eq!(sorted.next(), Some((PathBuf::from("5"), PathBuf::from("6")))); - assert_eq!(sorted.next(), Some((PathBuf::from("4"), PathBuf::from("5")))); - assert_eq!(sorted.next(), Some((PathBuf::from("3"), PathBuf::from("4")))); - assert_eq!(sorted.next(), Some((PathBuf::from("2"), PathBuf::from("3")))); - assert_eq!(sorted.next(), Some((PathBuf::from("1"), PathBuf::from("2")))); - assert_eq!(sorted.next(), None); + #[rustfmt::skip] + cmp( + &[("2", "3"), ("1", "2"), ("3", "4")], + &[("3", "4"), ("2", "3"), ("1", "2")] + ); + + #[rustfmt::skip] + cmp( + &[("1", "3"), ("2", "3"), ("3", "4")], + &[("3", "4"), ("2", "3"), ("1", "3")] + ); + + #[rustfmt::skip] + cmp( + &[("1", "2"), ("2", "1")], + &[("2", "1"), ("1", "2")] + ); } } From 212829370f8a15cffb5af883dd0e042f3695a7f1 Mon Sep 17 00:00:00 2001 From: Yongwoo Lee Date: Sun, 20 Oct 2024 12:17:51 +0900 Subject: [PATCH 4/7] feat: make result consistent & handle cyclic rename entries & add test contains valid rename and cycle --- yazi-core/src/manager/commands/bulk_rename.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/yazi-core/src/manager/commands/bulk_rename.rs b/yazi-core/src/manager/commands/bulk_rename.rs index c3b946609..c799ac2d5 100644 --- a/yazi-core/src/manager/commands/bulk_rename.rs +++ b/yazi-core/src/manager/commands/bulk_rename.rs @@ -142,9 +142,15 @@ impl Manager { }); if has_no_incomes.is_empty() { - todo!("Consider cycle") + // Remaining rename set has cycle, so we cannot sort, just return them all + let mut remain = todos.drain().collect::>(); + remain.sort(); + sorted.reverse(); + sorted.extend(remain); + return sorted; } + has_no_incomes.sort(); for old in has_no_incomes { income_map.remove(&old); let Some(new) = todos.remove(&old) else { unreachable!("") }; @@ -189,8 +195,14 @@ mod tests { #[rustfmt::skip] cmp( - &[("1", "2"), ("2", "1")], - &[("2", "1"), ("1", "2")] + &[("2", "1"), ("1", "2")], + &[("1", "2"), ("2", "1")] + ); + + #[rustfmt::skip] + cmp( + &[("3", "2"), ("2", "1"), ("1", "3"), ("a", "b"), ("b", "c")], + &[("b", "c"), ("a", "b"), ("1", "3"), ("2", "1"), ("3", "2")] ); } } From bbeeacfe7137842396a6c2a9dec8aac065dc5782 Mon Sep 17 00:00:00 2001 From: Yongwoo Lee Date: Tue, 22 Oct 2024 21:21:32 +0900 Subject: [PATCH 5/7] feat: reserve user input order --- yazi-core/src/manager/commands/bulk_rename.rs | 50 +++++++++++++------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/yazi-core/src/manager/commands/bulk_rename.rs b/yazi-core/src/manager/commands/bulk_rename.rs index c799ac2d5..973886170 100644 --- a/yazi-core/src/manager/commands/bulk_rename.rs +++ b/yazi-core/src/manager/commands/bulk_rename.rs @@ -1,12 +1,24 @@ -use std::{borrow::Cow, collections::HashMap, ffi::{OsStr, OsString}, io::{BufWriter, Write, stderr}, path::PathBuf}; - -use anyhow::{Result, anyhow}; +use std::{ + borrow::Cow, + collections::HashMap, + ffi::{OsStr, OsString}, + io::{stderr, BufWriter, Write}, + path::PathBuf, +}; + +use anyhow::{anyhow, Result}; use scopeguard::defer; -use tokio::{fs::{self, OpenOptions}, io::{AsyncReadExt, AsyncWriteExt, stdin}}; +use tokio::{ + fs::{self, OpenOptions}, + io::{stdin, AsyncReadExt, AsyncWriteExt}, +}; use yazi_config::{OPEN, PREVIEW}; use yazi_dds::Pubsub; -use yazi_proxy::{AppProxy, HIDER, TasksProxy, WATCHER}; -use yazi_shared::{fs::{File, FilesOp, Url, max_common_root, maybe_exists, paths_to_same_file}, terminal_clear}; +use yazi_proxy::{AppProxy, TasksProxy, HIDER, WATCHER}; +use yazi_shared::{ + fs::{max_common_root, maybe_exists, paths_to_same_file, File, FilesOp, Url}, + terminal_clear, +}; use crate::manager::Manager; @@ -119,16 +131,16 @@ impl Manager { } fn sort(old: Vec, new: Vec) -> Vec<(PathBuf, PathBuf)> { - let mut income_map: HashMap = - old.iter().map(|path| (path.clone(), false)).collect(); - let mut todos: HashMap = old - .into_iter() + let user_order: HashMap<_, _> = old.iter().enumerate().map(|(idx, path)| (path, idx)).collect(); + let mut income_map: HashMap<_, _> = old.iter().map(|path| (path.clone(), false)).collect(); + let mut todos: HashMap<_, _> = old + .iter() .zip(new) .map(|(old, new)| { if let Some(has_income) = income_map.get_mut(&new) { *has_income = true; } - (old, new) + (old.clone(), new) }) .collect(); @@ -144,13 +156,13 @@ impl Manager { if has_no_incomes.is_empty() { // Remaining rename set has cycle, so we cannot sort, just return them all let mut remain = todos.drain().collect::>(); - remain.sort(); + remain.sort_by(|(a, _), (b, _)| user_order[a].cmp(&user_order[b])); sorted.reverse(); sorted.extend(remain); return sorted; } - has_no_incomes.sort(); + has_no_incomes.sort_by(|a, b| user_order[b].cmp(&user_order[a])); for old in has_no_incomes { income_map.remove(&old); let Some(new) = todos.remove(&old) else { unreachable!("") }; @@ -190,19 +202,25 @@ mod tests { #[rustfmt::skip] cmp( &[("1", "3"), ("2", "3"), ("3", "4")], - &[("3", "4"), ("2", "3"), ("1", "3")] + &[("3", "4"), ("1", "3"), ("2", "3")] ); #[rustfmt::skip] cmp( &[("2", "1"), ("1", "2")], - &[("1", "2"), ("2", "1")] + &[("2", "1"), ("1", "2")] ); #[rustfmt::skip] cmp( &[("3", "2"), ("2", "1"), ("1", "3"), ("a", "b"), ("b", "c")], - &[("b", "c"), ("a", "b"), ("1", "3"), ("2", "1"), ("3", "2")] + &[("b", "c"), ("a", "b"), ("3", "2"), ("2", "1"), ("1", "3")] + ); + + #[rustfmt::skip] + cmp( + &[("b", "b_"), ("a", "a_"), ("c", "c_")], + &[("b", "b_"), ("a", "a_"), ("c", "c_")], ); } } From d6ea209690ee1dd70a527348e8f393e2632fc1ec Mon Sep 17 00:00:00 2001 From: sxyazi Date: Mon, 28 Oct 2024 23:57:48 +0800 Subject: [PATCH 6/7] Run `rustfmt +nightly **/*.rs` to make the code more readable in reviewing --- yazi-core/src/manager/commands/bulk_rename.rs | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/yazi-core/src/manager/commands/bulk_rename.rs b/yazi-core/src/manager/commands/bulk_rename.rs index 973886170..e36c3411e 100644 --- a/yazi-core/src/manager/commands/bulk_rename.rs +++ b/yazi-core/src/manager/commands/bulk_rename.rs @@ -1,24 +1,12 @@ -use std::{ - borrow::Cow, - collections::HashMap, - ffi::{OsStr, OsString}, - io::{stderr, BufWriter, Write}, - path::PathBuf, -}; - -use anyhow::{anyhow, Result}; +use std::{borrow::Cow, collections::HashMap, ffi::{OsStr, OsString}, io::{BufWriter, Write, stderr}, path::PathBuf}; + +use anyhow::{Result, anyhow}; use scopeguard::defer; -use tokio::{ - fs::{self, OpenOptions}, - io::{stdin, AsyncReadExt, AsyncWriteExt}, -}; +use tokio::{fs::{self, OpenOptions}, io::{AsyncReadExt, AsyncWriteExt, stdin}}; use yazi_config::{OPEN, PREVIEW}; use yazi_dds::Pubsub; -use yazi_proxy::{AppProxy, TasksProxy, HIDER, WATCHER}; -use yazi_shared::{ - fs::{max_common_root, maybe_exists, paths_to_same_file, File, FilesOp, Url}, - terminal_clear, -}; +use yazi_proxy::{AppProxy, HIDER, TasksProxy, WATCHER}; +use yazi_shared::{fs::{File, FilesOp, Url, max_common_root, maybe_exists, paths_to_same_file}, terminal_clear}; use crate::manager::Manager; From c0d67428f41de8d09e3cce6eb2a03a749e3ce67f Mon Sep 17 00:00:00 2001 From: sxyazi Date: Tue, 29 Oct 2024 12:55:46 +0800 Subject: [PATCH 7/7] refactor: simplify the code --- yazi-core/src/manager/commands/bulk_rename.rs | 52 ++++++++----------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/yazi-core/src/manager/commands/bulk_rename.rs b/yazi-core/src/manager/commands/bulk_rename.rs index e36c3411e..17bda7571 100644 --- a/yazi-core/src/manager/commands/bulk_rename.rs +++ b/yazi-core/src/manager/commands/bulk_rename.rs @@ -53,7 +53,7 @@ impl Manager { return Ok(()); } - let todo = Self::sort(old, new); + let todo = Self::prioritized_paths(old, new); if todo.is_empty() { return Ok(()); } @@ -118,46 +118,38 @@ impl Manager { Ok(()) } - fn sort(old: Vec, new: Vec) -> Vec<(PathBuf, PathBuf)> { - let user_order: HashMap<_, _> = old.iter().enumerate().map(|(idx, path)| (path, idx)).collect(); - let mut income_map: HashMap<_, _> = old.iter().map(|path| (path.clone(), false)).collect(); + fn prioritized_paths(old: Vec, new: Vec) -> Vec<(PathBuf, PathBuf)> { + let orders: HashMap<_, _> = old.iter().enumerate().map(|(i, p)| (p, i)).collect(); + let mut incomes: HashMap<_, _> = old.iter().map(|p| (p, false)).collect(); let mut todos: HashMap<_, _> = old .iter() .zip(new) - .map(|(old, new)| { - if let Some(has_income) = income_map.get_mut(&new) { - *has_income = true; - } - (old.clone(), new) + .map(|(o, n)| { + incomes.get_mut(&n).map(|b| *b = true); + (o, n) }) .collect(); - let mut sorted = vec![]; + let mut sorted = Vec::with_capacity(old.len()); while !todos.is_empty() { - let mut has_no_incomes = vec![]; - income_map.iter().for_each(|(old, has_income)| { - if !has_income { - has_no_incomes.push(old.clone()) - } - }); - - if has_no_incomes.is_empty() { - // Remaining rename set has cycle, so we cannot sort, just return them all - let mut remain = todos.drain().collect::>(); - remain.sort_by(|(a, _), (b, _)| user_order[a].cmp(&user_order[b])); + // Paths that are non-incomes and don't need to be prioritized in this round + let mut outcomes: Vec<_> = incomes.iter().filter(|(_, &b)| !b).map(|(&p, _)| p).collect(); + outcomes.sort_unstable_by(|a, b| orders[b].cmp(&orders[a])); + + // If there're no outcomes, it means there are cycles in the renaming + if outcomes.is_empty() { + let mut remain: Vec<_> = todos.into_iter().map(|(o, n)| (o.clone(), n)).collect(); + remain.sort_unstable_by(|(a, _), (b, _)| orders[a].cmp(&orders[b])); sorted.reverse(); sorted.extend(remain); return sorted; } - has_no_incomes.sort_by(|a, b| user_order[b].cmp(&user_order[a])); - for old in has_no_incomes { - income_map.remove(&old); - let Some(new) = todos.remove(&old) else { unreachable!("") }; - if let Some(has_income) = income_map.get_mut(&new) { - *has_income = false; - } - sorted.push((old, new)); + for old in outcomes { + let Some(new) = todos.remove(old) else { unreachable!() }; + incomes.remove(&old); + incomes.get_mut(&new).map(|b| *b = false); + sorted.push((old.clone(), new)); } } sorted.reverse(); @@ -172,7 +164,7 @@ mod tests { #[test] fn test_sort() { fn cmp(input: &[(&str, &str)], expected: &[(&str, &str)]) { - let sorted = Manager::sort( + let sorted = Manager::prioritized_paths( input.iter().map(|&(o, _)| o.into()).collect(), input.iter().map(|&(_, n)| n.into()).collect(), );