Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/but-api/src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ pub mod json {
fn from(value: but_core::diff::CommitDetails) -> Self {
let but_core::diff::CommitDetails {
commit,
diff_with_first_parent,
changes,
line_stats,
conflict_entries,
} = value;

CommitDetails {
commit: commit.into(),
changes: diff_with_first_parent.into_iter().map(Into::into).collect(),
changes: changes.into_iter().map(Into::into).collect(),
line_stats,
conflict_entries,
}
Expand Down
7 changes: 4 additions & 3 deletions crates/but-core/src/diff/commit_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub struct CommitDetails {
/// The fully decoded commit.
pub commit: crate::CommitOwned,
/// The changes between the tree of the first parent and this commit.
pub diff_with_first_parent: Vec<crate::TreeChange>,
pub changes: Vec<crate::TreeChange>,
/// The stats of the changes, which are computed only when explicitly requested.
pub line_stats: Option<LineStats>,
/// Represents what was causing a particular commit to conflict when rebased.
Expand All @@ -30,7 +30,8 @@ impl CommitDetails {
pub fn from_commit_id(commit_id: gix::Id, line_stats: bool) -> anyhow::Result<Self> {
let repo = commit_id.repo;
let commit = repo.find_commit(commit_id)?;
let first_parent_commit_id = commit.parent_ids().map(|id| id.detach()).next();
let first_parent_commit_id =
super::diff_base_commit_id(commit.parent_ids().map(|id| id.detach()))?;

let changes =
crate::diff::TreeChanges::from_trees(repo, first_parent_commit_id, commit_id.detach())?;
Expand All @@ -42,7 +43,7 @@ impl CommitDetails {
let conflict_entries = commit.conflict_entries()?;
Ok(CommitDetails {
commit: commit.detach(),
diff_with_first_parent: changes.into_tree_changes(),
changes: changes.into_tree_changes(),
line_stats: line_stats.map(Into::into),
conflict_entries,
})
Expand Down
8 changes: 8 additions & 0 deletions crates/but-core/src/diff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ impl ModeFlags {
}
}
}
/// Select the commit/tree to diff against:
/// - root commits use `None` (empty tree),
/// - all non-root commits (including merge commits) use the first parent.
pub(crate) fn diff_base_commit_id(
parent_ids: impl Iterator<Item = gix::ObjectId>,
) -> anyhow::Result<Option<gix::ObjectId>> {
Ok(parent_ids.into_iter().next())
}

#[cfg(test)]
mod tests {
Expand Down
9 changes: 2 additions & 7 deletions crates/but-core/src/diff/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,8 @@ pub fn commit_changes_with_line_stats_by_worktree_dir(
repo: &gix::Repository,
commit_id: gix::ObjectId,
) -> anyhow::Result<TreeChanges> {
let parent_id = commit_id
.attach(repo)
.object()?
.into_commit()
.parent_ids()
.map(|id| id.detach())
.next();
let commit = commit_id.attach(repo).object()?.into_commit();
let parent_id = super::diff_base_commit_id(commit.parent_ids().map(|id| id.detach()))?;
let (changes, stats) = super::tree_changes_with_line_stats(repo, parent_id, commit_id)
.map(|(c, s)| (c.into_iter().map(Into::into).collect(), s.into()))?;
Ok(TreeChanges { changes, stats })
Expand Down
34 changes: 34 additions & 0 deletions crates/but-core/tests/core/diff/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -739,3 +739,37 @@ pub fn repo(name: &str) -> anyhow::Result<gix::Repository> {
gix::open::Options::isolated(),
)?)
}

#[test]
fn merge_commit_ui_diff_uses_first_parent() -> anyhow::Result<()> {
let repo =
but_testsupport::read_only_in_memory_scenario("merge-with-two-branches-line-offset")?;
let merge_commit_id = repo.rev_parse_single("merge")?.detach();
let actual =
but_core::diff::ui::commit_changes_with_line_stats_by_worktree_dir(&repo, merge_commit_id)?;
assert_eq!(actual.stats.lines_added, 9);
assert_eq!(actual.stats.lines_removed, 0);
assert_eq!(actual.stats.files_changed, 1);

let actual_diff = actual.try_to_unidiff(&repo, 0)?;
let actual_diff: &[u8] = actual_diff.as_ref();
assert_eq!(
actual_diff,
br#"--- a/file
+++ b/file
@@ -1,0 +1,9 @@
+1
+2
+3
+4
+5
+6
+7
+8
+9

"#
);

Ok(())
}
4 changes: 2 additions & 2 deletions crates/but/src/command/legacy/diff/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub(crate) fn commit(

if let Some(json_out) = out.for_json() {
let changes: Vec<JsonChange> = result
.diff_with_first_parent
.changes
.into_iter()
.filter(|change| path.as_ref().is_none_or(|p| p == &change.path))
.map(|change| {
Expand All @@ -120,7 +120,7 @@ pub(crate) fn commit(
let output = JsonDiffOutput { changes };
json_out.write_value(output)?;
} else if let Some(out) = out.for_human_or_shell() {
for change in result.diff_with_first_parent {
for change in result.changes {
if path.as_ref().is_none_or(|p| p == &change.path) {
let patch = but_api::legacy::diff::tree_change_diffs(ctx, change.clone().into())
.ok()
Expand Down
9 changes: 4 additions & 5 deletions crates/but/src/command/legacy/reword.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,12 @@ fn edit_commit_message_by_id(
prepare_provided_message(message, "commit message").unwrap_or_else(|| {
let changed_files = get_changed_files_from_commit_details(&commit_details);

let should_show_diff = show_diff_in_editor.should_show_diff(|| {
estimate_diff_blob_size(&commit_details.diff_with_first_parent, ctx)
})?;
let should_show_diff = show_diff_in_editor
.should_show_diff(|| estimate_diff_blob_size(&commit_details.changes, ctx))?;
let diff = should_show_diff
.then(|| {
commit_details
.diff_with_first_parent
.changes
.iter()
.map(|change| change.unified_diff(&*ctx.repo.get()?, 3))
.filter_map(|diff| diff.transpose())
Expand Down Expand Up @@ -174,7 +173,7 @@ fn get_changed_files_from_commit_details(
) -> Vec<String> {
let mut files = Vec::new();

for change in &commit_details.diff_with_first_parent {
for change in &commit_details.changes {
let status = match &change.status {
but_core::TreeStatus::Addition { .. } => "new file:",
but_core::TreeStatus::Deletion { .. } => "deleted:",
Expand Down
2 changes: 1 addition & 1 deletion crates/but/src/command/legacy/status/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ pub fn print_group(
&repo,
commit.short_id.clone(),
&commit.inner,
CommitChanges::Remote(&details.diff_with_first_parent),
CommitChanges::Remote(&details.changes),
CommitClassification::Upstream,
false,
show_files,
Expand Down
2 changes: 1 addition & 1 deletion crates/but/src/tui/diff_viewer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ impl DiffFileEntry {
but_api::diff::commit_details(ctx, commit_id, but_api::diff::ComputeLineStats::No)?;

result
.diff_with_first_parent
.changes
.into_iter()
.filter(|change| path_filter.as_ref().is_none_or(|p| p == &change.path))
.map(|change| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub fn list_commit_files(
gix::prelude::ObjectIdExt::attach(commit_id, &repo),
false,
)
.map(|d| d.diff_with_first_parent)
.map(|d| d.changes)
}

pub fn create_commit(
Expand Down
Loading