Skip to content

Commit

Permalink
Refactor git commit hooks
Browse files Browse the repository at this point in the history
- the typescript side drives the hook runs
- some very basic tests
  • Loading branch information
mtsgrd committed Jan 9, 2025
1 parent 06ebdc4 commit 9fbe393
Show file tree
Hide file tree
Showing 22 changed files with 489 additions and 46 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

53 changes: 42 additions & 11 deletions apps/desktop/src/lib/commit/CommitDialog.svelte
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
<script lang="ts">
import CommitMessageInput from './CommitMessageInput.svelte';
import { PostHogWrapper } from '$lib/analytics/posthog';
import ContextMenuItem from '$lib/components/contextmenu/ContextMenuItem.svelte';
import ContextMenuSection from '$lib/components/contextmenu/ContextMenuSection.svelte';
import { persistedCommitMessage } from '$lib/config/config';
import { cloudCommunicationFunctionality } from '$lib/config/uiFeatureFlags';
import { SyncedSnapshotService } from '$lib/history/syncedSnapshotService';
import { HooksService } from '$lib/hooks/hooksService';
import { showError } from '$lib/notifications/toasts';
import DropDownButton from '$lib/shared/DropDownButton.svelte';
import { intersectionObserver } from '$lib/utils/intersectionObserver';
import { BranchController } from '$lib/vbranches/branchController';
Expand All @@ -25,30 +28,63 @@
const { projectId, expanded, hasSectionsAfter }: Props = $props();
const branchController = getContext(BranchController);
const hooksService = getContext(HooksService);
const posthog = getContext(PostHogWrapper);
const syncedSnapshotService = getContext(SyncedSnapshotService);
const canTakeSnapshot = syncedSnapshotService.canTakeSnapshot;
const selectedOwnership = getContextStore(SelectedOwnership);
const stack = getContextStore(BranchStack);
const commitMessage = persistedCommitMessage(projectId, $stack.id);
const canShowCommitAndPublish = $derived($cloudCommunicationFunctionality && $canTakeSnapshot);
let commitMessageInput = $state<CommitMessageInput>();
let isCommitting = $state(false);
let commitMessageValid = $state(false);
let isInViewport = $state(false);
let commitAndPublish = $state(false);
let commitButton = $state<DropDownButton>();
async function commit() {
const message = $commitMessage;
isCommitting = true;
try {
await branchController.commitBranch($stack.id, message.trim(), $selectedOwnership.toString());
$commitMessage = '';
const message = $commitMessage;
const ownership = $selectedOwnership.toString();
if (commitAndPublish) {
syncedSnapshotService.takeSyncedSnapshot($stack.id);
try {
const preCommitHook = await hooksService.preCommit(projectId, ownership);
if (preCommitHook.status === 'failure') {
showError('Pre-commit hook failed', preCommitHook.error);
return; // Abort commit if hook failed.
}
await branchController.commit($stack.id, message.trim(), ownership);
} catch (err: unknown) {
showError('Failed to commit changes', err);
posthog.capture('Commit Failed', { error: err });
return;
} finally {
isCommitting = false;
}
// Run both without awaiting unless commit failed.
runPostCommitActions();
runPostCommitHook();
}
async function runPostCommitActions() {
// Clear the commit message editor.
commitMessage.set('');
// Publishing a snapshot seems to imply posting a bleep.
if (commitAndPublish) {
await syncedSnapshotService.takeSyncedSnapshot($stack.id);
}
}
async function runPostCommitHook() {
const postCommitHook = await hooksService.postCommit(projectId);
if (postCommitHook.status === 'failure') {
showError('Post-commit hook failed', postCommitHook.error);
}
}
function close() {
Expand All @@ -60,11 +96,6 @@
await tick();
commitMessageInput?.focus();
}
const canShowCommitAndPublish = $derived($cloudCommunicationFunctionality && $canTakeSnapshot);
let commitAndPublish = $state(false);
let commitButton = $state<DropDownButton>();
</script>

<div
Expand Down
21 changes: 21 additions & 0 deletions apps/desktop/src/lib/commit/CommitMessageInput.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
projectCommitGenerationExtraConcise,
projectCommitGenerationUseEmojis
} from '$lib/config/config';
import { HooksService } from '$lib/hooks/hooksService';
import { showError } from '$lib/notifications/toasts';
import { isFailure } from '$lib/result';
import DropDownButton from '$lib/shared/DropDownButton.svelte';
Expand Down Expand Up @@ -47,12 +48,14 @@
const stack = getContextStore(BranchStack);
const project = getContext(Project);
const promptService = getContext(PromptService);
const hooksService = getContext(HooksService);
const aiGenEnabled = projectAiGenEnabled(project.id);
const commitGenerationExtraConcise = projectCommitGenerationExtraConcise(project.id);
const commitGenerationUseEmojis = projectCommitGenerationUseEmojis(project.id);
let aiLoading = $state(false);
let hookRunning = $state(false);

Check failure on line 58 in apps/desktop/src/lib/commit/CommitMessageInput.svelte

View workflow job for this annotation

GitHub Actions / lint-node

'hookRunning' is assigned a value but never used. Allowed unused vars must match /^_/u
let aiConfigurationValid = $state(false);
let titleTextArea: HTMLTextAreaElement | undefined = $state();
Expand Down Expand Up @@ -134,6 +137,22 @@
aiLoading = false;
}
async function runMessageHook() {
hookRunning = true;
try {
const hook_result = await hooksService.message(project.id, commitMessage);
if (hook_result.status === 'message') {
commitMessage = hook_result.message;
} else if (hook_result.status === 'failure') {
showError('Message hook failed', hook_result.error);
}
} catch (err: unknown) {
showError('Message hook failed', err);
} finally {
hookRunning = false;
}
}
onMount(async () => {
aiConfigurationValid = await aiService.validateConfiguration();
});
Expand Down Expand Up @@ -237,6 +256,7 @@
}}
onblur={() => {
isTitleFocused = false;
runMessageHook();
}}
oninput={(e: Event & { currentTarget: EventTarget & HTMLTextAreaElement }) => {
const target = e.currentTarget;
Expand All @@ -263,6 +283,7 @@
}}
onblur={() => {
isDescriptionFocused = false;
runMessageHook();
}}
oninput={(e: Event & { currentTarget: EventTarget & HTMLTextAreaElement }) => {
const target = e.currentTarget;
Expand Down
41 changes: 41 additions & 0 deletions apps/desktop/src/lib/hooks/hooksService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import type { Tauri } from '$lib/backend/tauri';

export type HookStatus =
| {
status: 'success';
}
| {
status: 'message';
message: string;
}
| {
status: 'notfound';
}
| {
status: 'failure';
error: string;
};

export class HooksService {
constructor(private tauri: Tauri) {}

async preCommit(projectId: string, ownership: string | undefined = undefined) {
return await this.tauri.invoke<HookStatus>('pre_commit_hook', {
projectId,
ownership
});
}

async postCommit(projectId: string) {
return await this.tauri.invoke<HookStatus>('post_commit_hook', {
projectId
});
}

async message(projectId: string, message: string) {
return await this.tauri.invoke<HookStatus>('message_hook', {
projectId,
message
});
}
}
28 changes: 5 additions & 23 deletions apps/desktop/src/lib/vbranches/branchController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,32 +52,14 @@ export class BranchController {
}
}

async runHooks(stackId: string, ownership: string) {
await invoke<void>('run_hooks', {
async commit(branchId: string, message: string, ownership: string | undefined = undefined) {
await invoke<void>('commit_virtual_branch', {
projectId: this.projectId,
stackId,
branch: branchId,
message,
ownership
});
}

async commitBranch(branchId: string, message: string, ownership: string | undefined = undefined) {
try {
await invoke<void>('commit_virtual_branch', {
projectId: this.projectId,
branch: branchId,
message,
ownership
});
this.posthog.capture('Commit Successful');
} catch (err: any) {
if (err.code === 'errors.commit.signing_failed') {
showSignError(err);
} else {
showError('Failed to commit changes', err);
throw err;
}
this.posthog.capture('Commit Failed', err);
}
this.posthog.capture('Commit Successful');
}

async integrateUpstreamForSeries(
Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/src/routes/+layout.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
GitHubUserService
} from '$lib/forge/github/githubUserService';
import { octokitFromAccessToken } from '$lib/forge/github/octokit';
import { HooksService } from '$lib/hooks/hooksService';
import ToastController from '$lib/notifications/ToastController.svelte';
import { platformName } from '$lib/platform/platform';
import { DesktopDispatch, DesktopState } from '$lib/redux/store.svelte';
Expand Down Expand Up @@ -79,6 +80,7 @@
setContext(OrganizationService, organizationService);
setContext(CloudUserService, cloudUserService);
setContext(CloudProjectService, cloudProjectService);
setContext(HooksService, data.hooksService);
// Setters do not need to be reactive since `data` never updates
setSecretsService(data.secretsService);
Expand Down
5 changes: 4 additions & 1 deletion apps/desktop/src/routes/+layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Tauri } from '$lib/backend/tauri';
import { UpdaterService } from '$lib/backend/updater';
import { loadAppSettings } from '$lib/config/appSettings';
import { FileService } from '$lib/files/fileService';
import { HooksService } from '$lib/hooks/hooksService';
import { RemotesService } from '$lib/remotes/service';
import { RustSecretService } from '$lib/secrets/secretsService';
import { TokenMemoryService } from '$lib/stores/tokenMemoryService';
Expand Down Expand Up @@ -62,6 +63,7 @@ export const load: LayoutLoad = async () => {
const lineManagerFactory = new LineManagerFactory();
const stackingLineManagerFactory = new StackingLineManagerFactory();
const fileService = new FileService(tauri);
const hooksService = new HooksService(tauri);

return {
commandService,
Expand All @@ -82,6 +84,7 @@ export const load: LayoutLoad = async () => {
secretsService,
posthog,
tauri,
fileService
fileService,
hooksService
};
};
1 change: 1 addition & 0 deletions crates/gitbutler-branch-actions/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,5 @@ pub use branch::{

pub use integration::GITBUTLER_WORKSPACE_COMMIT_TITLE;

pub mod ownership;
pub mod stack;
36 changes: 36 additions & 0 deletions crates/gitbutler-branch-actions/src/ownership.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use std::path::PathBuf;

use anyhow::{anyhow, Result};
use gitbutler_diff::{DiffByPathMap, GitHunk};
use gitbutler_stack::BranchOwnershipClaims;
use itertools::Itertools;

pub fn filter_hunks_by_ownership(
diffs: &DiffByPathMap,
ownership: &BranchOwnershipClaims,
) -> Result<Vec<(PathBuf, Vec<GitHunk>)>> {
ownership
.claims
.iter()
.map(|claim| {
if let Some(diff) = diffs.get(&claim.file_path) {
let hunks = claim
.hunks
.iter()
.filter_map(|claimed_hunk| {
diff.hunks
.iter()
.find(|diff_hunk| {
claimed_hunk.start == diff_hunk.new_start
&& claimed_hunk.end == diff_hunk.new_start + diff_hunk.new_lines
})
.cloned()
})
.collect_vec();
Ok((claim.file_path.clone(), hunks))
} else {
Err(anyhow!("Claim not found in workspace diff"))
}
})
.collect::<Result<Vec<(_, Vec<_>)>>>()
}
12 changes: 7 additions & 5 deletions crates/gitbutler-branch-actions/src/virtual.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ pub(crate) fn reset_files(
) -> Result<()> {
ctx.assure_resolved()?;

let stack = ctx
let branch = ctx
.project()
.virtual_branches()
.list_stacks_in_workspace()
Expand All @@ -267,7 +267,7 @@ pub(crate) fn reset_files(
.with_context(|| {
format!("could not find applied branch with id {stack_id} to reset files from")
})?;
let claims: Vec<_> = stack
let claims: Vec<_> = branch
.ownership
.claims
.into_iter()
Expand Down Expand Up @@ -729,6 +729,7 @@ pub fn commit(
message: &str,
ownership: Option<&BranchOwnershipClaims>,
) -> Result<git2::Oid> {
let mut stack = ctx.project().virtual_branches().get_stack(stack_id)?;
// get the files to commit
let diffs = gitbutler_diff::workdir(ctx.repo(), get_workspace_head(ctx)?)?;
let statuses = get_applied_status_cached(ctx, None, &diffs)
Expand Down Expand Up @@ -781,8 +782,9 @@ pub fn commit(

let git_repository = ctx.repo();
let parent_commit = git_repository
.find_commit(branch.head())
.context(format!("failed to find commit {:?}", branch.head()))?;
.find_commit(stack.head())
.context(format!("failed to find commit {:?}", stack.head()))?;

let tree = git_repository
.find_tree(tree_oid)
.context(format!("failed to find tree {:?}", tree_oid))?;
Expand All @@ -807,7 +809,7 @@ pub fn commit(
};

let vb_state = ctx.project().virtual_branches();
branch.set_stack_head(ctx, commit_oid, Some(tree_oid))?;
stack.set_stack_head(ctx, commit_oid, Some(tree_oid))?;

crate::integration::update_workspace_commit(&vb_state, ctx)
.context("failed to update gitbutler workspace")?;
Expand Down
Loading

0 comments on commit 9fbe393

Please sign in to comment.