Fix drag to uncommit

Author: mtsgrd

apps/desktop/src/components/v3/FileListItemWrapper.svelte

@@ -69,8 +69,6 @@
 	let contextMenu = $state<ReturnType<typeof FileContextMenu>>();
 	let draggableEl: HTMLDivElement | undefined = $state();
 
-	const selectedChanges = $derived(idSelection.treeChanges(projectId, selectionId));
-
 	const previousTooltipText = $derived(
 		(change.status.subject as Rename).previousPath
 			? `${(change.status.subject as Rename).previousPath} →\n${change.path}`
@@ -97,17 +95,12 @@
 
 	const checkStatus = $derived(uncommittedService.fileCheckStatus(stackId, change.path));
 
-	function onContextMenu(e: MouseEvent) {
-		if (
-			selectedChanges &&
-			selectedChanges.current.isSuccess &&
-			idSelection.has(change.path, selectionId)
-		) {
-			const changes: TreeChange[] = selectedChanges.current.data;
+	async function onContextMenu(e: MouseEvent) {
+		const changes = await idSelection.treeChanges(projectId, selectionId);
+		if (idSelection.has(change.path, selectionId)) {
 			contextMenu?.open(e, { changes });
 			return;
 		}
-
 		contextMenu?.open(e, { changes: [change] });
 	}
 
@@ -126,7 +119,7 @@
 	use:draggableChips={{
 		label: getFilename(change.path),
 		filePath: change.path,
-		data: new ChangeDropData(change, uncommittedService, idSelection, selectionId, stackId || null),
+		data: new ChangeDropData(projectId, change, idSelection, selectionId, stackId || null),
 		viewportId: 'board-viewport',
 		selector: '.selected-draggable',
 		disabled: draggableDisabled,

apps/desktop/src/components/v3/UnifiedDiffView.svelte

@@ -77,9 +77,7 @@
 		!draggable || !['commit', 'worktree'].includes(selectionId.type)
 	);
 
-	const assignments = $derived(
-		uncommittedService.getAssignmentsByPath(stackId || null, change.path)
-	);
+	const assignments = $derived(uncommittedService.assignmentsByPath(stackId || null, change.path));
 
 	function filter(hunks: DiffHunk[]): DiffHunk[] {
 		if (selectionId.type !== 'worktree') return hunks;

apps/desktop/src/lib/commits/dropHandler.ts

@@ -1,4 +1,4 @@
-import { dropDataToDiffSpec } from '$lib/commits/utils';
+import { changesToDiffSpec } from '$lib/commits/utils';
 import {
 	ChangeDropData,
 	FileDropData,
@@ -48,15 +48,15 @@ export class StartCommitDzHandler implements DropzoneHandler {
 			(data instanceof HunkDropDataV3 && data.uncommitted)
 		);
 	}
-	ondrop(data: ChangeDropData | HunkDropDataV3): void {
+	async ondrop(data: ChangeDropData | HunkDropDataV3): Promise<void> {
 		const { projectId, stackId, branchName, uiState, uncommittedService } = this.args;
 
 		const projectState = uiState.project(projectId);
 		const stackState = stackId ? uiState.stack(stackId) : undefined;
 
 		uncommittedService.uncheckAll(null);
 		if (data instanceof ChangeDropData) {
-			for (const change of data.changes) {
+			for (const change of await data.treeChanges()) {
 				uncommittedService.checkFile(null, change.path);
 			}
 		} else if (data instanceof HunkDropDataV3) {
@@ -126,14 +126,15 @@ export class AmendCommitWithChangeDzHandler implements DropzoneHandler {
 			case 'commit': {
 				const sourceStackId = data.stackId;
 				const sourceCommitId = data.selectionId.commitId;
+				const changes = changesToDiffSpec(await data.treeChanges());
 				if (sourceStackId && sourceCommitId) {
 					const { replacedCommits } = await this.stackService.moveChangesBetweenCommits({
 						projectId: this.projectId,
 						destinationStackId: this.stackId,
 						destinationCommitId: this.commit.id,
 						sourceStackId,
 						sourceCommitId,
-						changes: dropDataToDiffSpec(data)
+						changes
 					});
 
 					// Update the project state to point to the new commit if needed.
@@ -147,16 +148,18 @@ export class AmendCommitWithChangeDzHandler implements DropzoneHandler {
 			case 'branch':
 				console.warn('Moving a branch into a commit is an invalid operation');
 				break;
-			case 'worktree':
-				this.onresult(
+			case 'worktree': {
+				const diffSpec = changesToDiffSpec(await data.treeChanges());
+				return this.onresult(
 					await this.trigger({
 						projectId: this.projectId,
 						stackId: this.stackId,
 						branchName: this.branchName,
 						commitId: this.commit.id,
-						worktreeChanges: dropDataToDiffSpec(data)
+						worktreeChanges: diffSpec
 					})
 				);
+			}
 		}
 	}
 }
@@ -191,11 +194,12 @@ export class UncommitDzHandler implements DropzoneHandler {
 					const stackId = data.stackId;
 					const commitId = data.selectionId.commitId;
 					if (stackId && commitId) {
+						const changes = changesToDiffSpec(await data.treeChanges());
 						const { replacedCommits } = await this.stackService.uncommitChanges({
 							projectId: this.projectId,
 							stackId,
 							commitId,
-							changes: dropDataToDiffSpec(data)
+							changes
 						});
 
 						// Update the project state to point to the new commit if needed.

apps/desktop/src/lib/commits/utils.ts

@@ -1,5 +1,4 @@
 import type { DetailedCommit } from '$lib/commits/commit';
-import type { ChangeDropData } from '$lib/dragging/draggables';
 import type { TreeChange } from '$lib/hunks/change';
 import type { DiffSpec } from '$lib/hunks/hunk';
 
@@ -47,8 +46,3 @@ export function changesToDiffSpec(changes: TreeChange[]): DiffSpec[] {
 		};
 	});
 }
-/** Helper function that converts `ChangeDropData` to `DiffSpec`. */
-export function dropDataToDiffSpec(data: ChangeDropData): DiffSpec[] {
-	const changes = data.changes;
-	return changesToDiffSpec(changes);
-}

apps/desktop/src/lib/dragging/draggables.ts

@@ -1,12 +1,11 @@
-import { key, readKey, type SelectionId } from '$lib/selection/key';
+import { key, type SelectionId } from '$lib/selection/key';
 import { get, type Readable } from 'svelte/store';
 import type { AnyCommit } from '$lib/commits/commit';
 import type { CommitDropData } from '$lib/commits/dropHandler';
 import type { AnyFile } from '$lib/files/file';
 import type { TreeChange } from '$lib/hunks/change';
 import type { Hunk, HunkHeader, HunkLock } from '$lib/hunks/hunk';
 import type { IdSelection } from '$lib/selection/idSelection.svelte';
-import type { UncommittedService } from '$lib/selection/uncommittedService.svelte';
 
 export const NON_DRAGGABLE = {
 	disabled: true
@@ -38,8 +37,8 @@ export class HunkDropDataV3 {
 
 export class ChangeDropData {
 	constructor(
+		private projectId: string,
 		readonly change: TreeChange,
-		private uncommittedService: UncommittedService,
 		/**
 		 * When a a file is dragged we compare it to what is already selected,
 		 * if dragged item is part of the selection we consider that to be to
@@ -60,19 +59,17 @@ export class ChangeDropData {
 		}
 	}
 
-	get filePaths(): string[] {
+	/**
+	 * If there is more than one selected item, and the item being dragged is
+	 * part of that selection, then a drop handler will take an action on the
+	 * whole selection. If, however, the item being dragged is not part of a
+	 * selection then any action should be taken on that item alone.
+	 */
+	async treeChanges(): Promise<TreeChange[]> {
 		if (this.selection.has(this.change.path, this.selectionId)) {
-			const selectionKeys = this.selection.keys(this.selectionId);
-			return selectionKeys.map((key) => readKey(key).path);
-		} else {
-			return [this.change.path];
+			return await this.selection.treeChanges(this.projectId, this.selectionId);
 		}
-	}
-
-	get changes(): TreeChange[] {
-		const paths = this.filePaths;
-		const changes = this.uncommittedService.changesByStackId(this.stackId);
-		return changes.current.filter((change) => paths.includes(change.path));
+		return [this.change];
 	}
 
 	get isCommitted(): boolean {

apps/desktop/src/lib/hunks/dropHandler.ts

@@ -28,13 +28,13 @@ export class AssignmentDropHandler implements DropzoneHandler {
 
 	async ondrop(data: ChangeDropData | HunkDropDataV3) {
 		if (data instanceof ChangeDropData) {
+			const changes = await data.treeChanges();
+			const assignments = changes
+				.flatMap((c) => this.uncommittedService.getAssignmentsByPath(data.stackId, c.path))
+				.map((h) => ({ ...h, stackId: this.stackId }));
 			await this.diffService.assignHunk({
 				projectId: this.projectId,
-				assignments: data.changes
-					.flatMap(
-						(c) => this.uncommittedService.getAssignmentsByPath(data.stackId, c.path).current
-					)
-					.map((h) => ({ ...h, stackId: this.stackId }))
+				assignments
 			});
 		} else {
 			const assignment = this.uncommittedService.getAssignmentByHeader(

apps/desktop/src/lib/selection/idSelection.svelte.ts

@@ -7,8 +7,9 @@ import {
 	type SelectedFile
 } from '$lib/selection/key';
 import { SvelteSet } from 'svelte/reactivity';
+import type { TreeChange } from '$lib/hunks/change';
+import type { UncommittedService } from '$lib/selection/uncommittedService.svelte';
 import type { StackService } from '$lib/stacks/stackService.svelte';
-import type { WorktreeService } from '$lib/worktree/worktreeService.svelte';
 
 /**
  * File selection mechanism based on strings id's.
@@ -25,11 +26,11 @@ export class IdSelection {
 	>;
 
 	constructor(
-		private worktreeService: WorktreeService,
-		private stackService: StackService
+		private stackService: StackService,
+		private uncommittedService: UncommittedService
 	) {
 		this.selections = new Map();
-		this.selections.set('worktree', {
+		this.selections.set(selectionKey({ type: 'worktree' }), {
 			entries: new SvelteSet<SelectedFileKey>()
 		});
 	}
@@ -100,23 +101,27 @@ export class IdSelection {
 	 * instead reuses the entry from listing if available.
 	 * TODO: Should this be able to load even if listing hasn't happened?
 	 */
-	treeChanges(projectId: string, params: SelectionId) {
-		const filePaths = this.values(params).map((fileSelection) => {
+	async treeChanges(projectId: string, params: SelectionId): Promise<TreeChange[]> {
+		const paths = this.values(params).map((fileSelection) => {
 			return fileSelection.path;
 		});
 
 		switch (params.type) {
 			case 'worktree':
-				return this.worktreeService.treeChangesByPaths(projectId, filePaths);
+				return this.uncommittedService
+					.changesByStackId(params.stackId || null)
+					.current.filter((c) => paths.includes(c.path));
 			case 'branch':
-				return this.stackService.branchChangesByPaths({
+				return await this.stackService.branchChangesByPaths({
 					projectId,
 					stackId: params.stackId,
 					branchName: params.branchName,
-					paths: filePaths
+					paths: paths
 				});
 			case 'commit':
-				return this.stackService.commitChangesByPaths(projectId, params.commitId, filePaths);
+				return await this.stackService.commitChangesByPaths(projectId, params.commitId, paths);
+			case 'snapshot':
+				throw new Error('unsupported');
 		}
 	}
 

apps/desktop/src/lib/selection/key.ts

@@ -101,7 +101,7 @@ export function selectionKey(id: SelectionId): SelectedFileKey {
 		case 'branch':
 			return `${id.type}:${id.stackId}:${id.branchName}` as SelectedFileKey;
 		case 'worktree':
-			return `${id.type}` as SelectedFileKey;
+			return `${id.type}:${id.stackId}` as SelectedFileKey;
 		case 'snapshot':
 			return `${id.type}:${id.before}:${id.after}` as SelectedFileKey;
 	}

apps/desktop/src/lib/selection/uncommitted.ts

@@ -48,14 +48,10 @@ export const uncommittedSlice = createSlice({
 			);
 			const removedKeys = removedAssignments.map((r) => compositeKey(r));
 			if (removedKeys.length > 0) {
-				state.hunkAssignments = hunkAssignmentAdapter.removeMany(
-					state.hunkAssignments,
-					removedKeys
-				);
 				// The next line requires that assignments and selections share keys.
 				state.hunkSelection = hunkSelectionAdapter.removeMany(state.hunkSelection, removedKeys);
 			}
-			state.hunkAssignments = hunkAssignmentAdapter.upsertMany(
+			state.hunkAssignments = hunkAssignmentAdapter.addMany(
 				hunkAssignmentAdapter.getInitialState(),
 				action.payload.assignments
 			);

apps/desktop/src/lib/selection/uncommittedService.svelte.ts

@@ -166,16 +166,6 @@ export class UncommittedService {
 		return reactive(() => changes);
 	}
 
-	assignmentsByPath(stackId: string | null, path: string): Reactive<HunkAssignment[]> {
-		const result = $derived(
-			uncommittedSelectors.hunkAssignments.selectByPrefix(
-				this.state.hunkAssignments,
-				stackId + '-' + path + '-'
-			)
-		);
-		return reactive(() => result);
-	}
-
 	assignmentsByStackId(stackId: string | null): Reactive<HunkAssignment[]> {
 		const result = $derived(
 			uncommittedSelectors.hunkAssignments.selectByPrefix(this.state.hunkAssignments, stackId + '-')
@@ -193,13 +183,15 @@ export class UncommittedService {
 		return reactive(() => assignments);
 	}
 
-	getAssignmentsByPath(stackId: string | null, path: string): Reactive<HunkAssignment[]> {
-		const assignments = $derived(
-			uncommittedSelectors.hunkAssignments.selectByPrefix(
-				this.state.hunkAssignments,
-				partialKey(stackId, path)
-			)
+	getAssignmentsByPath(stackId: string | null, path: string): HunkAssignment[] {
+		return uncommittedSelectors.hunkAssignments.selectByPrefix(
+			this.state.hunkAssignments,
+			partialKey(stackId, path)
 		);
+	}
+
+	assignmentsByPath(stackId: string | null, path: string): Reactive<HunkAssignment[]> {
+		const assignments = $derived(this.getAssignmentsByPath(stackId, path));
 		return reactive(() => assignments);
 	}