Skip to content

Commit 33ec2f2

Browse files
vdavidvdavid
committed
Reduce structural code duplication
- Extract `start_write_operation` helper in write ops `mod.rs`, replacing 4x identical spawn/cleanup lifecycle - Extract `visible_entries` iterator helper in listing `operations.rs`, replacing 12 if-include_hidden branches and fixing O(n) collect-then-slice in `get_file_range` - Add `IoResultExt` trait with `.with_path()` for `io::Result` → `WriteOperationError::IoError` mapping across 7 write op files - Add `FileEntry::new()` constructor with sensible defaults, simplifying 20+ construction sites across production and test code - Add `with_savepoint` helper in `store.rs`, deduplicating savepoint boilerplate in batch insert and upsert - Replace 13 near-identical event handlers in `SearchDialog.svelte` with `inputHandler`/`selectHandler` factories
1 parent b97028f commit 33ec2f2

27 files changed

+667
-917
lines changed

‎apps/desktop/src-tauri/src/file_system/listing/caching_test.rs‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,12 @@ use super::sorting::{DirectorySortMode, SortColumn, SortOrder};
1212
/// Creates a minimal test entry.
1313
fn make_entry(name: &str, is_dir: bool, size: Option<u64>) -> FileEntry {
1414
FileEntry {
15-
name: name.to_string(),
16-
path: format!("/test/{}", name),
17-
is_directory: is_dir,
18-
is_symlink: false,
1915
size,
20-
physical_size: None,
21-
modified_at: None,
22-
created_at: None,
23-
added_at: None,
24-
opened_at: None,
2516
permissions: if is_dir { 0o755 } else { 0o644 },
2617
owner: "test".to_string(),
2718
group: "staff".to_string(),
28-
icon_id: if is_dir { "dir".to_string() } else { "file".to_string() },
2919
extended_metadata_loaded: true,
30-
recursive_size: None,
31-
recursive_physical_size: None,
32-
recursive_file_count: None,
33-
recursive_dir_count: None,
20+
..FileEntry::new(name.to_string(), format!("/test/{}", name), is_dir, false)
3421
}
3522
}
3623

‎apps/desktop/src-tauri/src/file_system/listing/hidden_files_test.rs‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,14 @@ use std::sync::Arc;
1616
/// Creates a test entry with the given name.
1717
fn make_entry(name: &str, is_dir: bool) -> FileEntry {
1818
FileEntry {
19-
name: name.to_string(),
20-
path: format!("/{}", name),
21-
is_directory: is_dir,
22-
is_symlink: false,
2319
size: if is_dir { None } else { Some(100) },
24-
physical_size: None,
2520
modified_at: Some(1_700_000_000),
2621
created_at: Some(1_700_000_000),
27-
added_at: None,
28-
opened_at: None,
2922
permissions: if is_dir { 0o755 } else { 0o644 },
3023
owner: "testuser".to_string(),
3124
group: "staff".to_string(),
32-
icon_id: if is_dir { "dir".to_string() } else { "file".to_string() },
3325
extended_metadata_loaded: true,
34-
recursive_size: None,
35-
recursive_physical_size: None,
36-
recursive_file_count: None,
37-
recursive_dir_count: None,
26+
..FileEntry::new(name.to_string(), format!("/{}", name), is_dir, false)
3827
}
3928
}
4029

‎apps/desktop/src-tauri/src/file_system/listing/metadata.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,33 @@ pub struct FileEntry {
105105
pub recursive_dir_count: Option<u64>,
106106
}
107107

108+
impl FileEntry {
109+
/// Creates a `FileEntry` with the four essential fields set and everything else defaulted.
110+
pub(crate) fn new(name: String, path: String, is_dir: bool, is_symlink: bool) -> Self {
111+
Self {
112+
icon_id: get_icon_id(is_dir, is_symlink, &name),
113+
name,
114+
path,
115+
is_directory: is_dir,
116+
is_symlink,
117+
size: None,
118+
physical_size: None,
119+
modified_at: None,
120+
created_at: None,
121+
added_at: None,
122+
opened_at: None,
123+
permissions: 0,
124+
owner: String::new(),
125+
group: String::new(),
126+
extended_metadata_loaded: false,
127+
recursive_size: None,
128+
recursive_physical_size: None,
129+
recursive_file_count: None,
130+
recursive_dir_count: None,
131+
}
132+
}
133+
}
134+
108135
/// Default value for extended_metadata_loaded (for backwards compatibility)
109136
fn default_extended_loaded() -> bool {
110137
true

‎apps/desktop/src-tauri/src/file_system/listing/operations.rs‎

Lines changed: 39 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@ fn is_visible(entry: &FileEntry) -> bool {
2121
!entry.name.starts_with('.')
2222
}
2323

24+
fn visible_entries<'a>(entries: &'a [FileEntry], include_hidden: bool) -> Box<dyn Iterator<Item = &'a FileEntry> + 'a> {
25+
if include_hidden {
26+
Box::new(entries.iter())
27+
} else {
28+
Box::new(entries.iter().filter(|e| is_visible(e)))
29+
}
30+
}
31+
2432
// ============================================================================
2533
// Listing lifecycle
2634
// ============================================================================
@@ -82,12 +90,7 @@ pub fn list_directory_start_with_volume(
8290
// Generate listing ID
8391
let listing_id = Uuid::new_v4().to_string();
8492

85-
// Count visible entries based on include_hidden setting
86-
let total_count = if include_hidden {
87-
all_entries.len()
88-
} else {
89-
all_entries.iter().filter(|e| is_visible(e)).count()
90-
};
93+
let total_count = visible_entries(&all_entries, include_hidden).count();
9194

9295
// Enrich directory entries with index data (recursive_size etc.) before sorting,
9396
// so that sort-by-size works correctly for directories.
@@ -165,19 +168,11 @@ pub fn get_file_range(
165168
.get(listing_id)
166169
.ok_or_else(|| format!("Listing not found: {}", listing_id))?;
167170

168-
// Filter entries if not including hidden
169-
// Cache entries are already enriched by the path that stored them (streaming,
170-
// watcher update, re-sort). Index freshness is handled separately by
171-
// `index-dir-updated` → `refreshIndexSizes` → `getDirStatsBatch`.
172-
let entries: Vec<FileEntry> = if include_hidden {
173-
let end = (start + count).min(listing.entries.len());
174-
listing.entries[start..end].to_vec()
175-
} else {
176-
// Need to filter and then slice
177-
let visible: Vec<&FileEntry> = listing.entries.iter().filter(|e| is_visible(e)).collect();
178-
let end = (start + count).min(visible.len());
179-
visible[start..end].iter().cloned().cloned().collect()
180-
};
171+
let entries: Vec<FileEntry> = visible_entries(&listing.entries, include_hidden)
172+
.skip(start)
173+
.take(count)
174+
.cloned()
175+
.collect();
181176

182177
Ok(entries)
183178
}
@@ -190,11 +185,7 @@ pub fn get_total_count(listing_id: &str, include_hidden: bool) -> Result<usize,
190185
.get(listing_id)
191186
.ok_or_else(|| format!("Listing not found: {}", listing_id))?;
192187

193-
if include_hidden {
194-
Ok(listing.entries.len())
195-
} else {
196-
Ok(listing.entries.iter().filter(|e| is_visible(e)).count())
197-
}
188+
Ok(visible_entries(&listing.entries, include_hidden).count())
198189
}
199190

200191
/// Gets the maximum filename width for a cached listing.
@@ -210,18 +201,10 @@ pub fn get_max_filename_width(listing_id: &str, include_hidden: bool) -> Result<
210201

211202
let font_id = "system-400-12"; // Default font (must match list_directory_start_with_volume)
212203

213-
let max_width = if include_hidden {
214-
let filenames: Vec<&str> = listing.entries.iter().map(|e| e.name.as_str()).collect();
215-
crate::font_metrics::calculate_max_width(&filenames, font_id)
216-
} else {
217-
let filenames: Vec<&str> = listing
218-
.entries
219-
.iter()
220-
.filter(|e| is_visible(e))
221-
.map(|e| e.name.as_str())
222-
.collect();
223-
crate::font_metrics::calculate_max_width(&filenames, font_id)
224-
};
204+
let filenames: Vec<&str> = visible_entries(&listing.entries, include_hidden)
205+
.map(|e| e.name.as_str())
206+
.collect();
207+
let max_width = crate::font_metrics::calculate_max_width(&filenames, font_id);
225208

226209
Ok(max_width)
227210
}
@@ -234,13 +217,7 @@ pub fn find_file_index(listing_id: &str, name: &str, include_hidden: bool) -> Re
234217
.get(listing_id)
235218
.ok_or_else(|| format!("Listing not found: {}", listing_id))?;
236219

237-
if include_hidden {
238-
Ok(listing.entries.iter().position(|e| e.name == name))
239-
} else {
240-
// Find index in filtered list
241-
let visible: Vec<&FileEntry> = listing.entries.iter().filter(|e| is_visible(e)).collect();
242-
Ok(visible.iter().position(|e| e.name == name))
243-
}
220+
Ok(visible_entries(&listing.entries, include_hidden).position(|e| e.name == name))
244221
}
245222

246223
/// Finds the indices of multiple files by name in a cached listing (batch version of `find_file_index`).
@@ -260,13 +237,7 @@ pub fn find_file_indices(
260237
let lookup: std::collections::HashSet<&str> = names.iter().map(|n| n.as_str()).collect();
261238
let mut result = HashMap::with_capacity(names.len());
262239

263-
let entries: Box<dyn Iterator<Item = &FileEntry>> = if include_hidden {
264-
Box::new(listing.entries.iter())
265-
} else {
266-
Box::new(listing.entries.iter().filter(|e| is_visible(e)))
267-
};
268-
269-
for (idx, entry) in entries.enumerate() {
240+
for (idx, entry) in visible_entries(&listing.entries, include_hidden).enumerate() {
270241
if lookup.contains(entry.name.as_str()) {
271242
result.insert(entry.name.clone(), idx);
272243
}
@@ -283,28 +254,16 @@ pub fn get_file_at(listing_id: &str, index: usize, include_hidden: bool) -> Resu
283254
.get(listing_id)
284255
.ok_or_else(|| format!("Listing not found: {}", listing_id))?;
285256

286-
if include_hidden {
287-
let result = listing.entries.get(index).cloned();
288-
if result.is_none() {
289-
log::error!(
290-
"get_file_at: index {} out of bounds (listing has {} entries) - frontend/backend index mismatch!",
291-
index,
292-
listing.entries.len()
293-
);
294-
}
295-
Ok(result)
296-
} else {
297-
let visible: Vec<&FileEntry> = listing.entries.iter().filter(|e| is_visible(e)).collect();
298-
let result = visible.get(index).cloned().cloned();
299-
if result.is_none() {
300-
log::error!(
301-
"get_file_at: index {} out of bounds (listing has {} visible entries) - frontend/backend index mismatch!",
302-
index,
303-
visible.len()
304-
);
305-
}
306-
Ok(result)
257+
let result = visible_entries(&listing.entries, include_hidden).nth(index).cloned();
258+
if result.is_none() {
259+
let total = visible_entries(&listing.entries, include_hidden).count();
260+
log::error!(
261+
"get_file_at: index {} out of bounds (listing has {} entries) - frontend/backend index mismatch!",
262+
index,
263+
total
264+
);
307265
}
266+
Ok(result)
308267
}
309268

310269
/// Gets file paths at specific indices from a cached listing.
@@ -322,12 +281,7 @@ pub fn get_paths_at_indices(
322281
.get(listing_id)
323282
.ok_or_else(|| format!("Listing not found: {}", listing_id))?;
324283

325-
// Build visible entries view (with or without hidden files)
326-
let visible: Vec<&FileEntry> = if include_hidden {
327-
listing.entries.iter().collect()
328-
} else {
329-
listing.entries.iter().filter(|e| is_visible(e)).collect()
330-
};
284+
let visible: Vec<&FileEntry> = visible_entries(&listing.entries, include_hidden).collect();
331285

332286
let mut paths = Vec::with_capacity(selected_indices.len());
333287
for &frontend_idx in selected_indices {
@@ -392,11 +346,7 @@ pub fn resort_listing(
392346
None
393347
} else {
394348
selected_indices.map(|indices| {
395-
let entries_for_index = if include_hidden {
396-
listing.entries.iter().collect::<Vec<_>>()
397-
} else {
398-
listing.entries.iter().filter(|e| is_visible(e)).collect()
399-
};
349+
let entries_for_index: Vec<_> = visible_entries(&listing.entries, include_hidden).collect();
400350
indices
401351
.iter()
402352
.filter_map(|&idx| entries_for_index.get(idx).map(|e| e.name.clone()))
@@ -414,34 +364,16 @@ pub fn resort_listing(
414364
listing.sort_order = sort_order;
415365

416366
// Find the new cursor position
417-
let new_cursor_index = cursor_filename.and_then(|name| {
418-
if include_hidden {
419-
listing.entries.iter().position(|e| e.name == name)
420-
} else {
421-
listing
422-
.entries
423-
.iter()
424-
.filter(|e| is_visible(e))
425-
.position(|e| e.name == name)
426-
}
427-
});
367+
let new_cursor_index =
368+
cursor_filename.and_then(|name| visible_entries(&listing.entries, include_hidden).position(|e| e.name == name));
428369

429370
// Find new indices of selected files
430371
let new_selected_indices = if all_selected {
431-
// All files are still selected after re-sort
432-
let count = if include_hidden {
433-
listing.entries.len()
434-
} else {
435-
listing.entries.iter().filter(|e| is_visible(e)).count()
436-
};
372+
let count = visible_entries(&listing.entries, include_hidden).count();
437373
Some((0..count).collect())
438374
} else {
439375
selected_filenames.map(|filenames| {
440-
let entries_for_lookup: Vec<_> = if include_hidden {
441-
listing.entries.iter().collect()
442-
} else {
443-
listing.entries.iter().filter(|e| is_visible(e)).collect()
444-
};
376+
let entries_for_lookup: Vec<_> = visible_entries(&listing.entries, include_hidden).collect();
445377
filenames
446378
.iter()
447379
.filter_map(|name| entries_for_lookup.iter().position(|e| e.name == *name))
@@ -551,20 +483,15 @@ pub fn get_listing_stats(
551483
.get(listing_id)
552484
.ok_or_else(|| format!("Listing not found: {}", listing_id))?;
553485

554-
// Get visible entries based on include_hidden setting
555-
let visible_entries: Vec<&FileEntry> = if include_hidden {
556-
listing.entries.iter().collect()
557-
} else {
558-
listing.entries.iter().filter(|e| is_visible(e)).collect()
559-
};
486+
let visible: Vec<&FileEntry> = visible_entries(&listing.entries, include_hidden).collect();
560487

561488
// Calculate totals
562489
let mut total_files: usize = 0;
563490
let mut total_dirs: usize = 0;
564491
let mut total_size: u64 = 0;
565492
let mut total_physical_size: u64 = 0;
566493

567-
for entry in &visible_entries {
494+
for entry in &visible {
568495
if entry.is_directory {
569496
total_dirs += 1;
570497
if let Some(size) = entry.recursive_size {
@@ -593,7 +520,7 @@ pub fn get_listing_stats(
593520
let mut sel_physical_size: u64 = 0;
594521

595522
for &idx in indices {
596-
if let Some(entry) = visible_entries.get(idx) {
523+
if let Some(entry) = visible.get(idx) {
597524
if entry.is_directory {
598525
sel_dirs += 1;
599526
if let Some(size) = entry.recursive_size {

0 commit comments

Comments
 (0)