-
Notifications
You must be signed in to change notification settings - Fork 728
feat: add ImageDiff component to load and show before/after images #11456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d311099
72da0e7
0aa478f
151dd9c
1c20615
878d1e3
1955e95
83489e5
08c9a14
ee26491
ccc131e
2608e18
f1331e8
7f8806f
f3633f4
83f391b
45b972d
35dca4b
c0d2682
d5249ad
4df7c2b
018291c
5e63987
8501a2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,197 @@ | ||||||
| <script lang="ts"> | ||||||
| import emptyFileSvg from '$lib/assets/empty-state/empty-file.svg?raw'; | ||||||
| import { FILE_SERVICE } from '$lib/files/fileService'; | ||||||
| import { inject } from '@gitbutler/core/context'; | ||||||
| import { ImageDiff, EmptyStatePlaceholder } from '@gitbutler/ui'; | ||||||
| import type { TreeChange } from '$lib/hunks/change'; | ||||||
| type Props = { | ||||||
| projectId: string; | ||||||
| change: TreeChange; | ||||||
| /** If provided, this is a commit diff (not a workspace diff). */ | ||||||
| commitId?: string; | ||||||
| }; | ||||||
| type ImageSource = | ||||||
| | { type: 'workspace'; path: string } | ||||||
| | { type: 'commit'; path: string; commitId: string } | ||||||
| | { type: 'blob'; path: string; blobId: string }; | ||||||
| type LoadStrategy = { | ||||||
| before: ImageSource | null; | ||||||
| after: ImageSource | null; | ||||||
| }; | ||||||
| const { projectId, change, commitId }: Props = $props(); | ||||||
| const fileService = inject(FILE_SERVICE); | ||||||
| let beforeImageUrl = $state<string | null>(null); | ||||||
| let afterImageUrl = $state<string | null>(null); | ||||||
| let loadError = $state<string | null>(null); | ||||||
| let isLoading = $state<boolean>(true); | ||||||
| // Decide image sources for before/after panels without changing logic. | ||||||
| function getLoadStrategy(): LoadStrategy { | ||||||
| const { status, path } = change; | ||||||
| const isCommitDiff = !!commitId; | ||||||
| switch (status.type) { | ||||||
| case 'Addition': | ||||||
| return isCommitDiff | ||||||
| ? { before: null, after: { type: 'commit' as const, path, commitId: commitId! } } | ||||||
| : { before: null, after: { type: 'workspace' as const, path } }; | ||||||
| case 'Deletion': | ||||||
| return isCommitDiff | ||||||
| ? { before: { type: 'commit' as const, path, commitId: `${commitId}^` }, after: null } | ||||||
| : { | ||||||
| before: { type: 'blob' as const, path, blobId: status.subject.previousState.id }, | ||||||
| after: null | ||||||
| }; | ||||||
| case 'Modification': | ||||||
| return isCommitDiff | ||||||
| ? { | ||||||
| before: { type: 'commit' as const, path, commitId: `${commitId}^` }, | ||||||
| after: { type: 'commit' as const, path, commitId: commitId! } | ||||||
| } | ||||||
| : { | ||||||
| before: { type: 'blob' as const, path, blobId: status.subject.previousState.id }, | ||||||
| after: { type: 'workspace' as const, path } | ||||||
| }; | ||||||
| case 'Rename': | ||||||
| return isCommitDiff | ||||||
| ? { | ||||||
| before: { | ||||||
| type: 'commit' as const, | ||||||
| path: status.subject.previousPath, | ||||||
| commitId: `${commitId}^` | ||||||
| }, | ||||||
| after: { type: 'commit' as const, path, commitId: commitId! } | ||||||
| } | ||||||
| : { | ||||||
| before: { | ||||||
| type: 'blob' as const, | ||||||
| path: status.subject.previousPath, | ||||||
| blobId: status.subject.previousState.id | ||||||
| }, | ||||||
| after: { type: 'workspace' as const, path } | ||||||
| }; | ||||||
| } | ||||||
| } | ||||||
|
Comment on lines
+34
to
+82
|
||||||
| /** | ||||||
| * Load image from workspace, commit, or blob. | ||||||
| * | ||||||
| * @param source - The image source to load. | ||||||
| * @param signal - Optional AbortSignal. Note: The backend service methods | ||||||
| * (readFromWorkspace, readFromCommit, readFromBlob) do NOT support AbortSignal, | ||||||
| * so the actual IO operations cannot be cancelled once started. The signal is | ||||||
| * only used to prevent processing results after abortion, not to abort the IO itself. | ||||||
| * | ||||||
| * @returns A data URL string or null. | ||||||
| * | ||||||
| * @remarks | ||||||
| * This function cannot cancel backend IO operations. If abort support is needed, | ||||||
| * the backend service methods must be updated to accept and honor AbortSignal. | ||||||
| * See: https://github.com/gitbutlerapp/gitbutler/issues (file a follow-up if needed) | ||||||
| */ | ||||||
| async function loadImage( | ||||||
| source: ImageSource | null, | ||||||
| signal?: AbortSignal | ||||||
| ): Promise<string | null> { | ||||||
| if (!source) return null; | ||||||
| try { | ||||||
| let fileInfo; | ||||||
| if (source.type === 'workspace') { | ||||||
| const { data } = await fileService.readFromWorkspace(source.path, projectId); | ||||||
| fileInfo = data; | ||||||
| } else if (source.type === 'commit') { | ||||||
| fileInfo = await fileService.readFromCommit(source.path, projectId, source.commitId); | ||||||
| } else { | ||||||
| // type === 'blob' | ||||||
| fileInfo = await fileService.readFromBlob(source.path, projectId, source.blobId); | ||||||
PavelLaptev marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| } | ||||||
| if (signal?.aborted) return null; | ||||||
| if (fileInfo?.content && fileInfo?.mimeType) { | ||||||
| return `data:${fileInfo.mimeType};base64,${fileInfo.content}`; | ||||||
| } | ||||||
| return null; | ||||||
| } catch (err) { | ||||||
| if (signal?.aborted) return null; | ||||||
| console.warn(`Failed to load image from ${source.type}: ${source.path}`, err); | ||||||
| return null; | ||||||
| } | ||||||
| } | ||||||
|
Comment on lines
+100
to
+131
|
||||||
| // Load both images according to the strategy. | ||||||
| async function loadImages(signal?: AbortSignal) { | ||||||
| isLoading = true; | ||||||
| loadError = null; | ||||||
| beforeImageUrl = null; | ||||||
| afterImageUrl = null; | ||||||
| try { | ||||||
| const strategy = getLoadStrategy(); | ||||||
| const [before, after] = await Promise.all([ | ||||||
| loadImage(strategy.before, signal), | ||||||
| loadImage(strategy.after, signal) | ||||||
| ]); | ||||||
| if (signal?.aborted) return; | ||||||
| beforeImageUrl = before; | ||||||
| afterImageUrl = after; | ||||||
| if (!before && !after) { | ||||||
| loadError = 'Failed to load both images (before and after).'; | ||||||
| } else if (!before && strategy.before) { | ||||||
| loadError = 'Failed to load before image.'; | ||||||
| } else if (!after && strategy.after) { | ||||||
| loadError = 'Failed to load after image.'; | ||||||
| } | ||||||
| } catch (err) { | ||||||
| console.error('Failed to load images:', err); | ||||||
| loadError = `Failed to load images: ${err instanceof Error ? err.message : String(err)}`; | ||||||
| } finally { | ||||||
| isLoading = false; | ||||||
| } | ||||||
| } | ||||||
| $effect(() => { | ||||||
| const abortController = new AbortController(); | ||||||
| loadImages(abortController.signal); | ||||||
| return () => abortController.abort(); | ||||||
| }); | ||||||
| </script> | ||||||
|
|
||||||
| {#if loadError} | ||||||
| <div class="imagediff-placeholder"> | ||||||
| <EmptyStatePlaceholder image={emptyFileSvg} gap={12} topBottomPadding={34}> | ||||||
| {#snippet caption()} | ||||||
| Can't preview this file type | ||||||
|
||||||
| Can't preview this file type | |
| {loadError} |
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I avoided using
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's a good call. The However, I don't think that anything but full hashes may be supported here. The frontend already has blob hashes for anything one would want to diff. Also I think there is a way to read an arbitrary file from within the worktree, in case of files that want comparison between worktree/Git. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| use std::path::PathBuf; | ||
|
|
||
| use crate::json::HexHash; | ||
| use anyhow::{Context as _, Result}; | ||
| use but_api_macros::but_api; | ||
| use but_core::DiffSpec; | ||
|
|
@@ -70,11 +71,10 @@ pub fn get_uncommitted_files(project_id: ProjectId) -> Result<Vec<RemoteBranchFi | |
| pub fn get_commit_file( | ||
| project_id: ProjectId, | ||
| relative_path: PathBuf, | ||
| commit_id: String, | ||
| commit_id: HexHash, | ||
| ) -> Result<FileInfo> { | ||
| let project = gitbutler_project::get(project_id)?; | ||
| let commit_id = git2::Oid::from_str(&commit_id).map_err(anyhow::Error::from)?; | ||
| project.read_file_from_commit(commit_id, &relative_path) | ||
| project.read_file_from_commit(commit_id.0.to_git2(), &relative_path) | ||
| } | ||
|
|
||
| #[but_api] | ||
|
|
@@ -84,6 +84,28 @@ pub fn get_workspace_file(project_id: ProjectId, relative_path: PathBuf) -> Resu | |
| project.read_file_from_workspace(&relative_path) | ||
| } | ||
|
|
||
| /// Retrieves file content directly from a Git blob object by its blob ID. | ||
| /// | ||
| /// This function is used for displaying image diff previews when the file | ||
| /// isn't available in the current workspace or a specific commit (e.g., for | ||
| /// deleted files or when comparing against a previous state). | ||
| /// | ||
| /// # Arguments | ||
| /// * `blob_id` - Git blob object ID as a hexadecimal string | ||
| #[but_api] | ||
| #[instrument(err(Debug))] | ||
| pub fn get_blob_file( | ||
| project_id: ProjectId, | ||
| relative_path: PathBuf, | ||
| blob_id: HexHash, | ||
| ) -> Result<FileInfo> { | ||
| let project = gitbutler_project::get(project_id)?; | ||
| let repo = project.open_isolated_repo()?; | ||
| let object = repo.find_object(blob_id).context("Failed to find blob")?; | ||
| let blob = object.try_into_blob().context("Object is not a blob")?; | ||
| Ok(FileInfo::from_content(&relative_path, &blob.data)) | ||
| } | ||
|
Comment on lines
+87
to
+107
|
||
|
|
||
| #[but_api] | ||
| #[instrument(err(Debug))] | ||
| pub fn pre_commit_hook( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,3 +40,21 @@ export function getFilePathInfo(filePath: string): FilePathInfo | undefined { | |
|
|
||
| return { fileName, extension, directoryPath }; | ||
| } | ||
|
|
||
| const IMAGE_EXTENSIONS = [ | ||
| '.png', | ||
| '.jpg', | ||
| '.jpeg', | ||
| '.gif', | ||
| '.webp', | ||
| '.bmp', | ||
| '.ico', | ||
| '.heic', | ||
| '.heif', | ||
| '.avif' | ||
| ]; | ||
|
|
||
| export function isImageFile(path: string): boolean { | ||
| const lowerPath = path.toLowerCase(); | ||
| return IMAGE_EXTENSIONS.some((ext) => lowerPath.endsWith(ext)); | ||
| } | ||
|
Comment on lines
+57
to
+60
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For commit diffs with deletions, the code constructs
${commitId}^to reference the parent commit. This assumes the commit has a parent, but the initial commit in a repository has no parent. While this is likely an edge case, consider adding error handling or documentation about this limitation. The backend will likely return an error for the initial commit, but the error message might not be user-friendly.