starred/nyaterm
1
0
Fork 0
mirror of https://github.com/nyakang/nyaterm.git synced 2026-11-15 08:08:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(sort): improve process and transfer entry sorting logic (#1065)

Browse Source 
- Extracted sorting logic into dedicated functions for better readability and maintainability.
- Updated sorting to handle NaN values correctly, ensuring consistent ordering.
- Added comprehensive tests for process sorting and transfer entry comparisons to validate sorting behavior across various scenarios.
- Enhanced dock zone detection logic to prioritize edges at equal distances and handle NaN distances appropriately.
This commit is contained in:
Kang
2026-11-03 19:06:21 +08:00
parent 58a7929644
commit a7f5382766
6 changed files with 502 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/nyaterm-desktop/src/features/remote/state/process.rs
+205 -37
View File
@@ -290,44 +290,43 @@ fn sort_processes(
key: RemoteProcessSortKey,
direction: RemoteProcessSortDirection,
) {
processes.sort_by(|left, right| {
let ordering = match key {
RemoteProcessSortKey::Command => left
.command
.cmp(&right.command)
.then_with(|| left.pid.cmp(&right.pid)),
RemoteProcessSortKey::Memory => left
.memory_percent
.partial_cmp(&right.memory_percent)
.unwrap_or(std::cmp::Ordering::Equal)
.then_with(|| {
left.rss_kb
.partial_cmp(&right.rss_kb)
.unwrap_or(std::cmp::Ordering::Equal)
})
.then_with(|| left.pid.cmp(&right.pid)),
RemoteProcessSortKey::Pid => left.pid.cmp(&right.pid),
RemoteProcessSortKey::User => left
.user
.cmp(&right.user)
.then_with(|| left.pid.cmp(&right.pid)),
RemoteProcessSortKey::Cpu => left
.cpu_percent
.partial_cmp(&right.cpu_percent)
.unwrap_or(std::cmp::Ordering::Equal)
.then_with(|| {
left.memory_percent
.partial_cmp(&right.memory_percent)
.unwrap_or(std::cmp::Ordering::Equal)
})
.then_with(|| left.pid.cmp(&right.pid)),
};
processes.sort_by(|left, right| compare_processes(left, right, key, direction));
}
match direction {
RemoteProcessSortDirection::Ascending => ordering,
RemoteProcessSortDirection::Descending => ordering.reverse(),
}
});
fn compare_processes(
left: &RemoteProcess,
right: &RemoteProcess,
key: RemoteProcessSortKey,
direction: RemoteProcessSortDirection,
) -> std::cmp::Ordering {
// Remote metrics may contain NaN. Treating unordered values as equal makes
// the secondary keys create cycles with otherwise ordered numeric values.
let ordering = match key {
RemoteProcessSortKey::Command => left
.command
.cmp(&right.command)
.then_with(|| left.pid.cmp(&right.pid)),
RemoteProcessSortKey::Memory => left
.memory_percent
.total_cmp(&right.memory_percent)
.then_with(|| left.rss_kb.cmp(&right.rss_kb))
.then_with(|| left.pid.cmp(&right.pid)),
RemoteProcessSortKey::Pid => left.pid.cmp(&right.pid),
RemoteProcessSortKey::User => left
.user
.cmp(&right.user)
.then_with(|| left.pid.cmp(&right.pid)),
RemoteProcessSortKey::Cpu => left
.cpu_percent
.total_cmp(&right.cpu_percent)
.then_with(|| left.memory_percent.total_cmp(&right.memory_percent))
.then_with(|| left.pid.cmp(&right.pid)),
};
match direction {
RemoteProcessSortDirection::Ascending => ordering,
RemoteProcessSortDirection::Descending => ordering.reverse(),
}
}
impl ProcessPaneState {
@@ -424,3 +423,172 @@ impl ProcessPaneState {
ProcessApplyOutcome::Applied { status }
}
}
#[cfg(test)]
mod sort_tests {
use std::cmp::Ordering;
use nyaterm_transport::RemoteProcess;
use super::{compare_processes, sort_processes};
use crate::models::{RemoteProcessSortDirection, RemoteProcessSortKey};
fn process(pid: u32, cpu: f64, memory: f64, rss: u64) -> RemoteProcess {
RemoteProcess {
pid,
ppid: 1,
user: format!("user{}", pid % 2),
state: "S".to_string(),
cpu_percent: cpu,
memory_percent: memory,
rss_kb: rss,
vsz_kb: 0,
elapsed: String::new(),
command: format!("command{}", pid % 3),
command_line: String::new(),
}
}
#[test]
fn nan_metrics_do_not_form_cycles_with_pid_ties() {
for key in [RemoteProcessSortKey::Cpu, RemoteProcessSortKey::Memory] {
let mut items = vec![
process(2, f64::NAN, f64::NAN, 0),
process(1, 1.0, 1.0, 0),
process(3, 0.0, 0.0, 0),
];
sort_processes(&mut items, key, RemoteProcessSortDirection::Ascending);
assert_eq!(
items.iter().map(|item| item.pid).collect::<Vec<_>>(),
[3, 1, 2]
);
sort_processes(&mut items, key, RemoteProcessSortDirection::Descending);
assert_eq!(
items.iter().map(|item| item.pid).collect::<Vec<_>>(),
[2, 1, 3]
);
}
}
#[test]
fn process_sort_modes_obey_ordering_laws_with_non_finite_metrics() {
let values = [
-f64::NAN,
f64::NEG_INFINITY,
-1.0,
-0.0,
0.0,
1.0,
f64::INFINITY,
f64::NAN,
];
let mut items = Vec::new();
for (cpu_index, cpu) in values.into_iter().enumerate() {
for (memory_index, memory) in values.into_iter().enumerate() {
items.push(process(
(cpu_index * values.len() + memory_index) as u32,
cpu,
memory,
(memory_index % 3) as u64,
));
}
}
items.push(items[0].clone());
for key in [
RemoteProcessSortKey::Cpu,
RemoteProcessSortKey::Memory,
RemoteProcessSortKey::Pid,
RemoteProcessSortKey::User,
RemoteProcessSortKey::Command,
] {
for direction in [
RemoteProcessSortDirection::Ascending,
RemoteProcessSortDirection::Descending,
] {
let orders: Vec<Vec<_>> = items
.iter()
.map(|left| {
items
.iter()
.map(|right| compare_processes(left, right, key, direction))
.collect()
})
.collect();
for a in 0..items.len() {
assert_eq!(orders[a][a], Ordering::Equal);
for b in 0..items.len() {
assert_eq!(orders[a][b], orders[b][a].reverse());
for c in 0..items.len() {
if orders[a][b].is_le() && orders[b][c].is_le() {
assert!(
orders[a][c].is_le(),
"non-transitive: {key:?}, {direction:?}, {a}/{b}/{c}"
);
}
if orders[a][b] == Ordering::Equal {
assert_eq!(orders[a][c], orders[b][c]);
}
}
}
}
let mut sorted = items.clone();
sorted.reverse();
sort_processes(&mut sorted, key, direction);
for (index, left) in sorted.iter().enumerate() {
for right in &sorted[index..] {
assert!(compare_processes(left, right, key, direction).is_le());
}
}
}
}
}
#[test]
fn metric_total_order_includes_infinities_and_signed_zero() {
let values = [
-f64::NAN,
f64::NEG_INFINITY,
-1.0,
-0.0,
0.0,
1.0,
f64::INFINITY,
f64::NAN,
];
for key in [RemoteProcessSortKey::Cpu, RemoteProcessSortKey::Memory] {
let mut items: Vec<_> = values
.into_iter()
.enumerate()
.map(|(index, value)| process(index as u32, value, value, 0))
.collect();
items.reverse();
sort_processes(&mut items, key, RemoteProcessSortDirection::Ascending);
assert_eq!(
items.iter().map(|item| item.pid).collect::<Vec<_>>(),
(0..8).collect::<Vec<_>>()
);
}
}
#[test]
fn process_metric_ties_keep_secondary_fields_and_pid_order() {
let mut items = vec![
process(4, 1.0, 3.0, 1),
process(3, 1.0, 2.0, 2),
process(2, 1.0, 2.0, 1),
process(1, 1.0, 2.0, 1),
];
for key in [RemoteProcessSortKey::Cpu, RemoteProcessSortKey::Memory] {
sort_processes(&mut items, key, RemoteProcessSortDirection::Ascending);
assert_eq!(
items.iter().map(|item| item.pid).collect::<Vec<_>>(),
[1, 2, 3, 4]
);
sort_processes(&mut items, key, RemoteProcessSortDirection::Descending);
assert_eq!(
items.iter().map(|item| item.pid).collect::<Vec<_>>(),
[4, 3, 2, 1]
);
}
}
}
crates/nyaterm-desktop/src/features/transfers/state/browser_logic.rs
+178 -12
View File
@@ -113,16 +113,11 @@ pub(in crate::features) fn compare_transfer_browser_entries(
column: TransferBrowserSortColumn,
direction: TransferBrowserSortDirection,
) -> Ordering {
if left.file_type != right.file_type {
let left_dir = left.is_directory();
let right_dir = right.is_directory();
if left_dir != right_dir {
return if left_dir {
Ordering::Less
} else {
Ordering::Greater
};
}
// Directory links and file links share a file type, but must still belong to
// different sort groups. Grouping cannot depend on the other entry's type.
let directory_order = right.is_directory().cmp(&left.is_directory());
if directory_order != Ordering::Equal {
return directory_order;
}
let result = match column {
@@ -232,8 +227,13 @@ pub(in crate::features) fn record_browser_history(
#[cfg(test)]
mod regression_tests {
use super::*;
use nyaterm_transport::SftpFileType;
use std::cmp::Ordering;
use std::sync::Arc;
use nyaterm_transport::{SftpFileEntry, SftpFileType};
use super::{BrowserFilterCache, BrowserFilterKey, compare_transfer_browser_entries};
use crate::models::{TransferBrowserSortColumn, TransferBrowserSortDirection};
fn entry(name: &str, file_type: SftpFileType, target_is_directory: bool) -> SftpFileEntry {
SftpFileEntry {
@@ -272,4 +272,170 @@ mod regression_tests {
"browser sorting comparator is not transitive"
);
}
const COLUMNS: [TransferBrowserSortColumn; 6] = [
TransferBrowserSortColumn::Name,
TransferBrowserSortColumn::Size,
TransferBrowserSortColumn::Modified,
TransferBrowserSortColumn::Permissions,
TransferBrowserSortColumn::Owner,
TransferBrowserSortColumn::Group,
];
const DIRECTIONS: [TransferBrowserSortDirection; 2] = [
TransferBrowserSortDirection::Ascending,
TransferBrowserSortDirection::Descending,
];
const KINDS: [(SftpFileType, bool); 5] = [
(SftpFileType::Directory, false),
(SftpFileType::Symlink, true),
(SftpFileType::Symlink, false),
(SftpFileType::File, false),
(SftpFileType::Other, false),
];
#[test]
fn all_browser_sort_modes_obey_ordering_laws_for_mixed_entries() {
let mut entries = Vec::new();
for (index, name) in ["", "a", "A", "m", "z", "file2", "file02", "file10", "文2"]
.into_iter()
.enumerate()
{
for (file_type, directory_link) in KINDS {
let mut item = entry(name, file_type, directory_link);
item.size = (index % 3 != 0).then_some((index % 4) as u64);
item.modified_at = (index % 3 != 0).then_some((index % 4) as u32);
item.permissions = (index % 3 != 0).then_some((index % 4) as u32);
item.owner = format!("owner{}", index % 3);
item.group = format!("group{}", index % 2);
entries.push(item);
}
}
// Missing metadata and explicit zero are intentionally equivalent.
let mut zero = entries[0].clone();
zero.size = Some(0);
zero.modified_at = Some(0);
zero.permissions = Some(0);
entries.push(zero);
for column in COLUMNS {
for direction in DIRECTIONS {
let orders: Vec<Vec<Ordering>> = entries
.iter()
.map(|left| {
entries
.iter()
.map(|right| {
compare_transfer_browser_entries(left, right, column, direction)
})
.collect()
})
.collect();
for (a, left) in entries.iter().enumerate() {
assert_eq!(orders[a][a], Ordering::Equal);
for (b, right) in entries.iter().enumerate() {
assert_eq!(orders[a][b], orders[b][a].reverse());
if left.is_directory() != right.is_directory() {
assert_eq!(orders[a][b].is_lt(), left.is_directory());
}
for c in 0..entries.len() {
if orders[a][b].is_le() && orders[b][c].is_le() {
assert!(
orders[a][c].is_le(),
"non-transitive: {column:?}, {direction:?}, {a}/{b}/{c}"
);
}
if orders[a][b] == Ordering::Equal {
assert_eq!(orders[a][c], orders[b][c]);
}
}
}
}
}
}
}
#[test]
fn browser_cache_sorts_large_mixed_listings_and_reuses_each_result() {
let entries = Arc::new(
(0..128)
.map(|offset| {
let index = (offset * 73) % 128;
let (file_type, directory_link) = KINDS[index % KINDS.len()];
let mut item = entry(&format!("entry{index}"), file_type, directory_link);
item.size = Some(index as u64);
item.modified_at = Some(index as u32);
item.permissions = Some(index as u32);
item.owner = format!("owner{index}");
item.group = format!("group{index}");
item
})
.collect::<Vec<_>>(),
);
let mut cache = BrowserFilterCache::default();
let mut recomputes = 0;
for column in COLUMNS {
for direction in DIRECTIONS {
let key = BrowserFilterKey::new(&entries, "", true, column, direction);
let sorted = cache.entries(key.clone(), &entries);
let mut expected = Vec::new();
for directory in [true, false] {
let mut indices: Vec<_> = (0..128)
.filter(|index| (*index % KINDS.len() < 2) == directory)
.collect();
if direction == TransferBrowserSortDirection::Descending {
indices.reverse();
}
expected.extend(indices.into_iter().map(|index| format!("entry{index}")));
}
assert_eq!(
sorted.iter().map(|item| &item.name).collect::<Vec<_>>(),
expected.iter().collect::<Vec<_>>(),
"{column:?}, {direction:?}",
);
recomputes += 1;
assert_eq!(cache.recomputes(), recomputes);
assert!(Arc::ptr_eq(&sorted, &cache.entries(key, &entries)));
assert_eq!(cache.recomputes(), recomputes);
}
}
}
#[test]
fn browser_sort_preserves_name_ties_and_missing_metadata_rules() {
let mut a = entry("file2", SftpFileType::File, false);
let mut b = entry("file10", SftpFileType::File, false);
for column in COLUMNS {
if column == TransferBrowserSortColumn::Name {
continue;
}
for direction in DIRECTIONS {
assert_eq!(
compare_transfer_browser_entries(&a, &b, column, direction),
Ordering::Less,
"name fallback stays ascending: {column:?}, {direction:?}",
);
}
}
a.size = Some(1);
b.size = Some(0);
assert_eq!(
compare_transfer_browser_entries(
&a,
&b,
TransferBrowserSortColumn::Size,
TransferBrowserSortDirection::Ascending
),
Ordering::Greater,
);
b.size = None;
assert_eq!(
compare_transfer_browser_entries(
&a,
&b,
TransferBrowserSortColumn::Size,
TransferBrowserSortDirection::Ascending
),
Ordering::Greater,
);
}
}
crates/nyaterm-desktop/src/models/workspace_tabs/groups.rs
+37
View File
@@ -238,6 +238,43 @@ mod tests {
SmartSplitMode, TabDockEdge, TabDockZone, TerminalWindowNode, WorkspacePaneNode,
WorkspaceSplitDirection,
};
#[test]
fn dock_zone_keeps_edge_priority_at_equal_distances() {
assert_eq!(
TabDockZone::detect(10., 10., 800., 600.),
TabDockZone::Edge(TabDockEdge::Left)
);
assert_eq!(
TabDockZone::detect(790., 10., 800., 600.),
TabDockZone::Edge(TabDockEdge::Right)
);
assert_eq!(
TabDockZone::detect(400., 10., 800., 600.),
TabDockZone::Edge(TabDockEdge::Top)
);
assert_eq!(
TabDockZone::detect(400., 590., 800., 600.),
TabDockZone::Edge(TabDockEdge::Bottom)
);
assert_eq!(
TabDockZone::detect(400., 300., 800., 600.),
TabDockZone::Center
);
}
#[test]
fn dock_zone_orders_nan_distances_after_finite_edges() {
assert_eq!(
TabDockZone::detect(f32::NAN, 10., 800., 600.),
TabDockZone::Edge(TabDockEdge::Top)
);
assert_eq!(
TabDockZone::detect(10., f32::NAN, 800., 600.),
TabDockZone::Edge(TabDockEdge::Left)
);
}
fn assert_leaf_sizes(tree: &TerminalWindowNode, width: f32, height: f32) {
match tree {
TerminalWindowNode::Leaf { .. } => {
crates/nyaterm-desktop/src/models/workspace_tabs/mod.rs
+1 -1
View File
@@ -75,7 +75,7 @@ impl TabDockZone {
(TabDockEdge::Top, local_y, v_thresh),
(TabDockEdge::Bottom, height - local_y, v_thresh),
];
edges.sort_by(|a, b| a.1.partial_cmp(&b.1).unwrap_or(std::cmp::Ordering::Equal));
edges.sort_by(|a, b| a.1.total_cmp(&b.1));
if let Some((edge, distance, threshold)) = edges.first().copied()
&& distance <= threshold
{
crates/nyaterm-transport/src/local_fs.rs
+59 -8
View File
@@ -46,12 +46,7 @@ impl LocalFileService {
entries.push(file_entry);
}
}
entries.sort_by(|left, right| {
(left.file_type != SftpFileType::Directory)
.cmp(&(right.file_type != SftpFileType::Directory))
.then_with(|| left.name.to_lowercase().cmp(&right.name.to_lowercase()))
.then_with(|| left.name.cmp(&right.name))
});
sort_entries(&mut entries);
Ok(entries)
}
@@ -332,6 +327,15 @@ fn ensure_safe_local_target(path: &Path) -> anyhow::Result<()> {
Ok(())
}
fn sort_entries(entries: &mut [SftpFileEntry]) {
entries.sort_by(|left, right| {
(!left.is_directory())
.cmp(&(!right.is_directory()))
.then_with(|| left.name.to_lowercase().cmp(&right.name.to_lowercase()))
.then_with(|| left.name.cmp(&right.name))
});
}
fn file_entry_from_path(path: &Path, name: String) -> anyhow::Result<SftpFileEntry> {
let symlink_metadata = fs::symlink_metadata(path)?;
let metadata = fs::metadata(path).unwrap_or_else(|_| symlink_metadata.clone());
@@ -511,13 +515,60 @@ fn set_local_mode_if_supported(_path: &Path, _mode: Option<u32>) -> anyhow::Resu
#[cfg(test)]
mod tests {
use super::LocalFileService;
use crate::{RemoteTextWriteResult, SftpFileType, SftpWriteTextResult};
use super::{LocalFileService, sort_entries};
use crate::{RemoteTextWriteResult, SftpFileEntry, SftpFileType, SftpWriteTextResult};
fn temp_dir(name: &str) -> nyaterm_core::test_support::TestTempDir {
nyaterm_core::test_support::TestTempDir::new(&format!("nyaterm-local-fs-{name}"))
}
#[test]
fn local_listing_groups_directory_links_before_files_and_preserves_name_order() {
let mut entries: Vec<_> = [
("a.txt", SftpFileType::File, false),
("m-file-link", SftpFileType::Symlink, false),
("z-dir-link", SftpFileType::Symlink, true),
("target-file.txt", SftpFileType::File, false),
("target-dir", SftpFileType::Directory, false),
("A.txt", SftpFileType::File, false),
("entry10", SftpFileType::Other, false),
("entry2", SftpFileType::File, false),
]
.into_iter()
.map(
|(name, file_type, symlink_target_is_directory)| SftpFileEntry {
name: name.to_string(),
path: name.to_string(),
file_type,
size: None,
permissions: None,
owner: String::new(),
group: String::new(),
modified_at: None,
raw_path_token: None,
symlink_target_is_directory,
},
)
.collect();
sort_entries(&mut entries);
assert_eq!(
entries
.iter()
.map(|item| item.name.as_str())
.collect::<Vec<_>>(),
[
"target-dir",
"z-dir-link",
"A.txt",
"a.txt",
"entry10",
"entry2",
"m-file-link",
"target-file.txt",
]
);
}
#[test]
fn local_service_lists_and_mutates_files() {
let service = LocalFileService;
crates/nyaterm-ui/src/menu.rs
+22 -1
View File
@@ -72,7 +72,7 @@ fn submenu_direction_probe_width(
.iter()
.filter_map(|item| item.children().map(|_| item.submenu_min_width))
.map(|width| width.unwrap_or(px(NYA_MENU_WIDTH)))
.max_by(|left, right| left.partial_cmp(right).unwrap_or(std::cmp::Ordering::Equal))?;
.max_by(|left, right| f32::from(*left).total_cmp(&f32::from(*right)))?;
let parent_width = parent_min_width.unwrap_or(px(NYA_MENU_WIDTH));
let parent_and_child = parent_width + child_width - px(NYA_SUBMENU_OVERLAP);
(pointer_x + parent_and_child > window_width).then_some(parent_and_child)
@@ -1149,6 +1149,27 @@ mod tests {
);
}
#[test]
fn context_submenu_uses_widest_child_independently_of_item_order() {
for widths in [[160., 240., 240., 176.], [240., 176., 160., 240.]] {
let items: Vec<_> = widths
.into_iter()
.map(|width| {
NyaMenuItem::submenu("Move", vec![NyaMenuItem::action("Folder")])
.submenu_min_width(px(width))
})
.collect();
assert_eq!(
submenu_direction_probe_width(&items, None, px(48.), px(381.)),
Some(px(452.))
);
assert_eq!(
submenu_direction_probe_width(&items, None, px(48.), px(900.)),
None
);
}
}
#[test]
fn context_submenu_uses_configured_parent_and_child_widths() {
let items = vec![