diff --git a/CONFIGURATION.md b/CONFIGURATION.md index c4824e4..32c302a 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -176,6 +176,19 @@ gitflow.release.finish.signingkey=ABC123DEF gitflow.hotfix.finish.keeplocal=true ``` +### Delete Command Options + +| Option | Description | Values | Default | +|--------|-------------|--------|---------| +| `fetch` | Fetch and fast-forward parent before delete | `true`, `false` | `false` | + +#### Examples + +```bash +# Always fetch before deleting feature branches +gitflow.feature.delete.fetch=true +``` + ### Update Command Options | Option | Description | Values | Default | diff --git a/cmd/delete.go b/cmd/delete.go index 25d2980..c408582 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -11,8 +11,8 @@ import ( ) // DeleteCommand handles the deletion of a topic branch -func DeleteCommand(branchType string, name string, force *bool, remote *bool) { - if err := executeDelete(branchType, name, force, remote); err != nil { +func DeleteCommand(branchType string, name string, force *bool, remote *bool, fetch *bool) { + if err := executeDelete(branchType, name, force, remote, fetch); err != nil { var exitCode errors.ExitCode if flowErr, ok := err.(errors.Error); ok { exitCode = flowErr.ExitCode() @@ -25,7 +25,7 @@ func DeleteCommand(branchType string, name string, force *bool, remote *bool) { } // executeDelete performs the actual branch deletion logic and returns any errors -func executeDelete(branchType string, name string, force *bool, remote *bool) error { +func executeDelete(branchType string, name string, force *bool, remote *bool, fetch *bool) error { // Load configuration cfg, err := config.LoadConfig() if err != nil { @@ -73,12 +73,38 @@ func executeDelete(branchType string, name string, force *bool, remote *bool) er // Run delete operation wrapped with hooks return hooks.WithHooks(gitDir, branchType, hooks.HookActionDelete, hookCtx, func() error { - return performDelete(branchType, name, fullBranchName, branchConfig, force, remote, cfg) + return performDelete(branchType, name, fullBranchName, branchConfig, force, remote, fetch, cfg) }) } // performDelete performs the actual delete operation (called within hooks wrapper) -func performDelete(branchType, name, fullBranchName string, branchConfig config.BranchConfig, force *bool, remote *bool, cfg *config.Config) error { +func performDelete(branchType, name, fullBranchName string, branchConfig config.BranchConfig, force *bool, remote *bool, fetch *bool, cfg *config.Config) error { + // Determine if we should fetch before deleting + shouldFetch := false + if fetch != nil { + shouldFetch = *fetch + } else { + configKey := fmt.Sprintf("gitflow.%s.delete.fetch", branchType) + fetchConfig, err := git.GetConfig(configKey) + if err == nil && fetchConfig == "true" { + shouldFetch = true + } + } + + if shouldFetch && git.RemoteExists(cfg.Remote) { + fmt.Printf("Fetching from remote '%s'...\n", cfg.Remote) + // Fetch parent branch + if err := git.FetchBranch(cfg.Remote, branchConfig.Parent); err != nil { + // Non-fatal: remote branch might not exist + fmt.Fprintf(os.Stderr, "Note: Could not fetch parent branch '%s': %v\n", branchConfig.Parent, err) + } + // Fetch topic branch + if err := git.FetchBranch(cfg.Remote, fullBranchName); err != nil { + // Non-fatal: remote branch might not exist (may have been deleted after merge) + fmt.Fprintf(os.Stderr, "Note: Could not fetch topic branch '%s': %v\n", fullBranchName, err) + } + } + // Determine if we should force delete the branch forceDelete := false if force != nil { @@ -93,6 +119,30 @@ func performDelete(branchType, name, fullBranchName string, branchConfig config. } } + // Check if local branch is in sync with remote (unless --force) + if shouldFetch && !forceDelete { + status, commitCount, err := git.CompareBranchWithRemote(fullBranchName) + if err == nil { // Only check if we can get tracking info + switch status { + case git.SyncStatusBehind, git.SyncStatusDiverged: + trackingBranch, err := git.GetTrackingBranch(fullBranchName) + if err != nil { + trackingBranch = "remote tracking branch" + } + return &errors.BranchBehindRemoteError{ + BranchName: fullBranchName, + RemoteBranch: trackingBranch, + CommitCount: commitCount, + BranchType: branchType, + } + case git.SyncStatusAhead: + fmt.Printf("Note: Local branch is %d commit(s) ahead of remote\n", commitCount) + } + // SyncStatusEqual or SyncStatusNoTracking - proceed normally + } + // If err != nil (no tracking branch), proceed normally + } + // Determine if we should delete remote branch deleteRemote := false if remote != nil { @@ -125,11 +175,23 @@ func performDelete(branchType, name, fullBranchName string, branchConfig config. if err := git.Checkout(parentBranch); err != nil { return &errors.GitError{Operation: fmt.Sprintf("checkout parent branch '%s'", parentBranch), Err: err} } + currentBranch = parentBranch } else { return &errors.GitError{Operation: "delete branch", Err: fmt.Errorf("cannot delete the current branch without a parent branch configured")} } } + // After fetch, fast-forward the parent branch so git branch -d can detect + // branches merged remotely (e.g., via GitHub PR merge). + // Only do this when we're on the parent branch, since git branch -d checks against HEAD. + if shouldFetch && currentBranch == branchConfig.Parent && git.RemoteExists(cfg.Remote) { + remoteBranch := fmt.Sprintf("%s/%s", cfg.Remote, branchConfig.Parent) + if err := git.MergeFFOnly(remoteBranch); err != nil { + // Non-fatal: parent may have diverged or remote branch may not exist + fmt.Fprintf(os.Stderr, "Note: Could not fast-forward '%s': %v\n", branchConfig.Parent, err) + } + } + // Delete the branch with appropriate flag deleteErr := git.DeleteBranch(fullBranchName, forceDelete) if deleteErr != nil { diff --git a/cmd/shorthand.go b/cmd/shorthand.go index 7bfd93a..90056d8 100644 --- a/cmd/shorthand.go +++ b/cmd/shorthand.go @@ -36,23 +36,16 @@ func RegisterShorthandCommands() { if err != nil { return err } - var force *bool - if cmd.Flags().Changed("force") { - f, _ := cmd.Flags().GetBool("force") - force = &f - } else if cmd.Flags().Changed("no-force") { - f := false - force = &f - } - var remote *bool - if cmd.Flags().Changed("remote") { - r, _ := cmd.Flags().GetBool("remote") - remote = &r - } else if cmd.Flags().Changed("no-remote") { - f := false - remote = &f - } - DeleteCommand(branchType, name, force, remote) + forceFlag, _ := cmd.Flags().GetBool("force") + noForceFlag, _ := cmd.Flags().GetBool("no-force") + force := getBoolFlag(forceFlag, noForceFlag) + remoteFlag, _ := cmd.Flags().GetBool("remote") + noRemoteFlag, _ := cmd.Flags().GetBool("no-remote") + remote := getBoolFlag(remoteFlag, noRemoteFlag) + fetchFlag, _ := cmd.Flags().GetBool("fetch") + noFetchFlag, _ := cmd.Flags().GetBool("no-fetch") + fetch := getBoolFlag(fetchFlag, noFetchFlag) + DeleteCommand(branchType, name, force, remote, fetch) return nil }, } @@ -60,6 +53,8 @@ func RegisterShorthandCommands() { deleteCmd.Flags().Bool("no-force", false, "Don't force delete (overrides config)") deleteCmd.Flags().BoolP("remote", "r", false, "Delete remote tracking branch") deleteCmd.Flags().Bool("no-remote", false, "Don't delete remote tracking branch") + deleteCmd.Flags().Bool("fetch", false, "Fetch from remote before deleting") + deleteCmd.Flags().Bool("no-fetch", false, "Don't fetch from remote before deleting") rootCmd.AddCommand(deleteCmd) // Update diff --git a/cmd/topicbranch.go b/cmd/topicbranch.go index b6ae9a8..47dd325 100644 --- a/cmd/topicbranch.go +++ b/cmd/topicbranch.go @@ -282,6 +282,8 @@ func registerBranchCommand(branchType string) { noForce, _ := cmd.Flags().GetBool("no-force") remote, _ := cmd.Flags().GetBool("remote") noRemote, _ := cmd.Flags().GetBool("no-remote") + fetch, _ := cmd.Flags().GetBool("fetch") + noFetch, _ := cmd.Flags().GetBool("no-fetch") // Convert force flags to a single *bool var forcePtr *bool @@ -301,7 +303,7 @@ func registerBranchCommand(branchType string) { remotePtr = &falseBool } - DeleteCommand(branchType, args[0], forcePtr, remotePtr) + DeleteCommand(branchType, args[0], forcePtr, remotePtr, getBoolFlag(fetch, noFetch)) return nil }, } @@ -311,6 +313,8 @@ func registerBranchCommand(branchType string) { deleteCmd.Flags().Bool("no-force", false, "Don't force delete the branch (overrides config)") deleteCmd.Flags().BoolP("remote", "r", false, "Delete the remote tracking branch") deleteCmd.Flags().Bool("no-remote", false, "Don't delete the remote tracking branch") + deleteCmd.Flags().Bool("fetch", false, "Fetch from remote before deleting") + deleteCmd.Flags().Bool("no-fetch", false, "Don't fetch from remote before deleting") branchCmd.AddCommand(deleteCmd) diff --git a/docs/git-flow-delete.1.md b/docs/git-flow-delete.1.md index afcc825..09709f1 100644 --- a/docs/git-flow-delete.1.md +++ b/docs/git-flow-delete.1.md @@ -38,6 +38,12 @@ The delete operation removes the specified topic branch from the local repositor **--no-remote** : Don't delete the remote tracking branch (default behavior) +**--fetch** +: Fetch from remote before deleting. This updates local refs so that Git can correctly detect whether the branch has been merged remotely (e.g., via a GitHub PR merge). + +**--no-fetch** +: Don't fetch from remote before deleting (overrides config) + ## SAFETY CHECKS By default, Git prevents deletion of branches with unmerged changes. The delete command: @@ -67,6 +73,13 @@ Delete with remote cleanup: git flow feature delete my-feature --remote ``` +### Fetch Before Delete + +Fetch and fast-forward the parent branch before deleting, so Git can detect branches merged remotely (e.g., via a GitHub PR merge): +```bash +git flow feature delete my-feature --fetch +``` + ### Force Deletion Delete branch with unmerged changes: @@ -144,6 +157,12 @@ git config gitflow.hotfix.delete.force true git config gitflow.branch.feature.deleteRemote true ``` +### Fetch Settings +```bash +# Always fetch before deleting feature branches +git config gitflow.feature.delete.fetch true +``` + ## SAFETY CONSIDERATIONS **Unmerged Changes** diff --git a/docs/gitflow-config.5.md b/docs/gitflow-config.5.md index 0058e72..d5eeeb1 100644 --- a/docs/gitflow-config.5.md +++ b/docs/gitflow-config.5.md @@ -455,6 +455,11 @@ The finish command supports extensive merge strategy configuration through comma : *Type*: boolean : *Default*: true +**gitflow.*type*.delete.fetch** +: Fetch and fast-forward the parent branch before deleting a topic branch. When enabled, updates the parent branch so that Git can correctly detect whether the topic branch has been merged remotely (e.g., via a GitHub PR merge). +: *Type*: boolean +: *Default*: false + ### Merge Message Options **gitflow.*type*.finish.mergemessage** diff --git a/internal/git/repo.go b/internal/git/repo.go index 95347a6..88d65dc 100644 --- a/internal/git/repo.go +++ b/internal/git/repo.go @@ -417,6 +417,17 @@ func RebaseWithOptions(targetBranch string, preserveMerges bool) error { return nil } +// MergeFFOnly attempts a fast-forward-only merge of the given branch into the current branch. +// Returns an error if the merge cannot be fast-forwarded. +func MergeFFOnly(branch string) error { + cmd := exec.Command("git", "merge", "--ff-only", branch) + output, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("fast-forward merge of %q failed: %w: %s", branch, err, strings.TrimSpace(string(output))) + } + return nil +} + // MergeWithOptions merges a branch into current branch with optional no-fast-forward func MergeWithOptions(branchName string, noFF bool, noVerify bool) error { args := []string{"merge"} diff --git a/test/cmd/delete_test.go b/test/cmd/delete_test.go index 7aedd30..1e7352e 100644 --- a/test/cmd/delete_test.go +++ b/test/cmd/delete_test.go @@ -880,3 +880,172 @@ func TestDeleteFeatureBranchRemoteNoRemoteError(t *testing.T) { t.Error("Expected feature/test-feature branch to still exist after failed delete --remote") } } + +// TestDeleteFeatureWithFetchFlag tests that the --fetch flag fetches before deleting. +// Steps: +// 1. Sets up a test repository with remote (includes git-flow init) +// 2. Starts a feature branch +// 3. Runs 'git flow feature delete my-feature --fetch' +// 4. Verifies output contains "Fetching from remote" +// 5. Verifies branch is deleted +func TestDeleteFeatureWithFetchFlag(t *testing.T) { + // Setup test repository with remote + dir, remoteDir := testutil.SetupTestRepoWithRemote(t) + defer testutil.CleanupTestRepo(t, dir) + defer testutil.CleanupTestRepo(t, remoteDir) + + // Create a feature branch + _, err := testutil.RunGitFlow(t, dir, "feature", "start", "test-fetch") + if err != nil { + t.Fatalf("Failed to create feature branch: %v", err) + } + + // Delete with --fetch flag + output, err := testutil.RunGitFlow(t, dir, "feature", "delete", "test-fetch", "--fetch") + if err != nil { + t.Fatalf("Failed to delete feature branch: %v\nOutput: %s", err, output) + } + + // Verify that fetch occurred + if !strings.Contains(output, "Fetching from remote") { + t.Error("Expected fetch to occur with --fetch flag") + } + + // Verify branch is deleted + if testutil.BranchExists(t, dir, "feature/test-fetch") { + t.Error("Expected feature branch to be deleted") + } +} + +// TestDeleteFeatureWithFetchConfig tests that gitflow.feature.delete.fetch config triggers fetch. +// Steps: +// 1. Sets up a test repository with remote (includes git-flow init) +// 2. Starts a feature branch +// 3. Sets gitflow.feature.delete.fetch to true +// 4. Runs 'git flow feature delete my-feature' (no flag) +// 5. Verifies output contains "Fetching from remote" +// 6. Verifies branch is deleted +func TestDeleteFeatureWithFetchConfig(t *testing.T) { + // Setup test repository with remote + dir, remoteDir := testutil.SetupTestRepoWithRemote(t) + defer testutil.CleanupTestRepo(t, dir) + defer testutil.CleanupTestRepo(t, remoteDir) + + // Create a feature branch + _, err := testutil.RunGitFlow(t, dir, "feature", "start", "test-fetch-config") + if err != nil { + t.Fatalf("Failed to create feature branch: %v", err) + } + + // Set fetch config + _, err = testutil.RunGit(t, dir, "config", "gitflow.feature.delete.fetch", "true") + if err != nil { + t.Fatalf("Failed to set config: %v", err) + } + + // Delete without flag (should use config) + output, err := testutil.RunGitFlow(t, dir, "feature", "delete", "test-fetch-config") + if err != nil { + t.Fatalf("Failed to delete feature branch: %v\nOutput: %s", err, output) + } + + // Verify that fetch occurred + if !strings.Contains(output, "Fetching from remote") { + t.Error("Expected fetch to occur with fetch config enabled") + } + + // Verify branch is deleted + if testutil.BranchExists(t, dir, "feature/test-fetch-config") { + t.Error("Expected feature branch to be deleted") + } +} + +// TestDeleteFeatureWithNoFetchOverridesConfig tests that --no-fetch overrides fetch config. +// Steps: +// 1. Sets up a test repository with remote (includes git-flow init) +// 2. Starts a feature branch +// 3. Sets gitflow.feature.delete.fetch to true +// 4. Runs 'git flow feature delete my-feature --no-fetch' +// 5. Verifies output does NOT contain "Fetching" +// 6. Verifies branch is deleted +func TestDeleteFeatureWithNoFetchOverridesConfig(t *testing.T) { + // Setup test repository with remote + dir, remoteDir := testutil.SetupTestRepoWithRemote(t) + defer testutil.CleanupTestRepo(t, dir) + defer testutil.CleanupTestRepo(t, remoteDir) + + // Create a feature branch + _, err := testutil.RunGitFlow(t, dir, "feature", "start", "test-no-fetch") + if err != nil { + t.Fatalf("Failed to create feature branch: %v", err) + } + + // Set fetch config + _, err = testutil.RunGit(t, dir, "config", "gitflow.feature.delete.fetch", "true") + if err != nil { + t.Fatalf("Failed to set config: %v", err) + } + + // Delete with --no-fetch flag (should override config) + output, err := testutil.RunGitFlow(t, dir, "feature", "delete", "test-no-fetch", "--no-fetch") + if err != nil { + t.Fatalf("Failed to delete feature branch: %v\nOutput: %s", err, output) + } + + // Verify that fetch did NOT occur + if strings.Contains(output, "Fetching") { + t.Error("Expected --no-fetch to override config and skip fetch") + } + + // Verify branch is deleted + if testutil.BranchExists(t, dir, "feature/test-no-fetch") { + t.Error("Expected feature branch to be deleted") + } +} + +// TestDeleteFeatureNoRemoteFetchSkipped tests that --fetch silently skips when no remote exists. +// Steps: +// 1. Sets up a test repository (no remote) and initializes git-flow with defaults +// 2. Starts a feature branch +// 3. Runs 'git flow feature delete my-feature --fetch' +// 4. Verifies output does NOT contain "Fetching" (fetch skipped silently) +// 5. Verifies no error about missing remote +// 6. Verifies branch is deleted +func TestDeleteFeatureNoRemoteFetchSkipped(t *testing.T) { + // Setup test repository without remote + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + // Initialize git-flow with defaults + _, err := testutil.RunGitFlow(t, dir, "init", "--defaults") + if err != nil { + t.Fatalf("Failed to initialize git-flow: %v", err) + } + + // Create a feature branch + _, err = testutil.RunGitFlow(t, dir, "feature", "start", "test-no-remote") + if err != nil { + t.Fatalf("Failed to create feature branch: %v", err) + } + + // Delete with --fetch flag (no remote configured) + output, err := testutil.RunGitFlow(t, dir, "feature", "delete", "test-no-remote", "--fetch") + if err != nil { + t.Fatalf("Failed to delete feature branch: %v\nOutput: %s", err, output) + } + + // Verify fetch was skipped silently + if strings.Contains(output, "Fetching") { + t.Error("Expected fetch to be skipped silently when no remote exists") + } + + // Verify no confusing error messages + if strings.Contains(output, "does not appear to be a git repository") { + t.Error("Expected no confusing error about missing remote") + } + + // Verify branch is deleted + if testutil.BranchExists(t, dir, "feature/test-no-remote") { + t.Error("Expected feature branch to be deleted") + } +}