Skip to content

Align merge commit diff behavior with first-parent semantics in but-core and update regression coverage#12796

Draft
Copilot wants to merge 2 commits intomasterfrom
copilot/gt-12795-add-test
Draft

Align merge commit diff behavior with first-parent semantics in but-core and update regression coverage#12796
Copilot wants to merge 2 commits intomasterfrom
copilot/gt-12795-add-test

Conversation

Copy link
Contributor

Copilot AI commented Mar 12, 2026

This PR aligns merge-commit diff behavior with current expected semantics: for commit views, diffs should follow git show/GitHub-style first-parent behavior rather than merge-base diffing.

A previous iteration switched merge commits to merge-base diffing, but follow-up review showed this does not match expected behavior for this codepath.

What changed

  • Updated diff-base selection in but-core::diff so that:
    • root commits diff against None (empty tree),
    • all non-root commits (including merge commits) diff against their first parent.
  • Kept both affected call sites consistent:
    • CommitDetails::from_commit_id()
    • ui::commit_changes_with_line_stats_by_worktree_dir()
  • Updated merge-commit regression coverage in crates/but-core/tests/core/diff/ui.rs to assert first-parent output and stats.

Result

Commit diff rendering for merge commits now matches first-parent semantics expected for this view, and tests guard against regressions in that behavior.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@vercel
Copy link

vercel bot commented Mar 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
gitbutler-web Ignored Ignored Preview Mar 12, 2026 7:12am

Request Review

Copilot AI changed the title [WIP] [GT-12795] Add test for issue resolution Use merge-base for merge commit diffs in but-core and add regression coverage Mar 12, 2026
@github-actions github-actions bot added the rust Pull requests that update Rust code label Mar 12, 2026
@Byron Byron force-pushed the copilot/gt-12795-add-test branch from bf2ad3b to 36d882e Compare March 12, 2026 06:12
@github-actions github-actions bot added the CLI The command-line program `but` label Mar 12, 2026
@Byron Byron force-pushed the copilot/gt-12795-add-test branch from 36d882e to 806c67d Compare March 12, 2026 06:16
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
@Byron Byron force-pushed the copilot/gt-12795-add-test branch from 806c67d to ed2cf5a Compare March 12, 2026 06:17
Copy link
Collaborator

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking the merge of the PR as there is open questions.
At least it's clear now that there is much to be learned and possibly implemented to deal with merges better.

Co-authored-by: Byron <63622+Byron@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 12, 2026

Blocking the merge of the PR as there is open questions.
At least it's clear now that there is much to be learned and ...

Updated in 28c909d: I reverted merge-commit diff-base behavior to first-parent semantics (matching git show/GitHub commit view), adjusted docs accordingly, and updated the regression test to assert first-parent output for merge commits.

Copilot AI changed the title Use merge-base for merge commit diffs in but-core and add regression coverage Align merge commit diff behavior with first-parent semantics in but-core and update regression coverage Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLI The command-line program `but` rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merge commit diffs show all incoming changes instead of merge-only changes

2 participants