Skip to content

Commit 1d50656

Browse files
but-hunk-assignment: clarify return value of assignments_with_fallback
As documented, this function seems to perform an operation based on the worktree changes and locks, but if the operation fails, falls back to using the worktree changes alone, implying that the operation may fail due to the locks. However, the only fallible part of the operation (`reconcile_with_worktree_and_locks`) is I/O error on `virtual_branches.toml` or database access. Falling back on I/O or database error doesn't seem like a good idea - it seems best to fail immediately. Therefore, remove this fallback. TODO: if we have a better name for `assignments_with_fallback`, we should inline `reconcile_worktree_changes_with_worktree_and_locks` and update all callers with the new name and signature before merging this commit.
1 parent ac86c81 commit 1d50656

File tree

1 file changed

+20
-9
lines changed
  • crates/but-hunk-assignment/src

1 file changed

+20
-9
lines changed

crates/but-hunk-assignment/src/lib.rs

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -319,16 +319,29 @@ pub fn assign(
319319
Ok(rejections)
320320
}
321321

322-
/// Same as the `reconcile_with_worktree_and_locks` function, but if the operation produces an error, it will create a fallback set of assignments from the worktree changes alone.
323-
/// An optional error is returned alongside the assignments, which will be `None` if the operation was successful and it will be set if the operation failed and a fallback was used.
324-
///
325-
/// The fallback can of course also fail, in which case the tauri operation will error out.
322+
/// Similar to the `reconcile_with_worktree_and_locks` function.
323+
/// TODO: figure out a better name for this function
326324
pub fn assignments_with_fallback(
327325
ctx: &mut Context,
328326
set_assignment_from_locks: bool,
329327
worktree_changes: Option<impl IntoIterator<Item = impl Into<but_core::TreeChange>>>,
330328
deps: Option<&HunkDependencies>,
331329
) -> Result<(Vec<HunkAssignment>, Option<anyhow::Error>)> {
330+
let hunk_assignments = reconcile_worktree_changes_with_worktree_and_locks(
331+
ctx,
332+
set_assignment_from_locks,
333+
worktree_changes,
334+
deps,
335+
)?;
336+
Ok((hunk_assignments, None))
337+
}
338+
339+
fn reconcile_worktree_changes_with_worktree_and_locks(
340+
ctx: &mut Context,
341+
set_assignment_from_locks: bool,
342+
worktree_changes: Option<impl IntoIterator<Item = impl Into<but_core::TreeChange>>>,
343+
deps: Option<&HunkDependencies>,
344+
) -> Result<Vec<HunkAssignment>> {
332345
let repo = ctx.repo.get()?;
333346
let worktree_changes: Vec<but_core::TreeChange> = match worktree_changes {
334347
Some(wtc) => wtc.into_iter().map(Into::into).collect(),
@@ -344,7 +357,7 @@ pub fn assignments_with_fallback(
344357
};
345358

346359
if worktree_changes.is_empty() {
347-
return Ok((vec![], None));
360+
return Ok(vec![]);
348361
}
349362
let mut worktree_assignments = vec![];
350363
for change in &worktree_changes {
@@ -360,12 +373,10 @@ pub fn assignments_with_fallback(
360373
set_assignment_from_locks,
361374
&worktree_assignments,
362375
deps,
363-
)
364-
.map(|a| (a, None))
365-
.unwrap_or_else(|e| (worktree_assignments, Some(e)));
376+
)?;
366377

367378
let db = &mut *ctx.db.get_mut()?;
368-
state::set_assignments(db, reconciled.0.clone())?;
379+
state::set_assignments(db, reconciled.clone())?;
369380
Ok(reconciled)
370381
}
371382

0 commit comments

Comments
 (0)