From f902b234930a12a73e054a39ed7d49283e5b209d Mon Sep 17 00:00:00 2001 From: Joshix Date: Tue, 20 Jun 2023 18:00:00 +0000 Subject: [PATCH 1/4] fix sorting of commits in diff view --- asyncgit/src/commit_files.rs | 11 +++- asyncgit/src/diff.rs | 7 +- asyncgit/src/sync/commit_files.rs | 66 +++++++++++++------ asyncgit/src/sync/diff.rs | 14 ++-- .../commit_details/compare_details.rs | 44 ++++++------- src/components/commit_details/mod.rs | 8 ++- src/components/compare_commits.rs | 15 +++-- 7 files changed, 103 insertions(+), 62 deletions(-) diff --git a/asyncgit/src/commit_files.rs b/asyncgit/src/commit_files.rs index c02e6e8f15..fbc5b65e3d 100644 --- a/asyncgit/src/commit_files.rs +++ b/asyncgit/src/commit_files.rs @@ -1,6 +1,6 @@ use crate::{ error::Result, - sync::{self, CommitId, RepoPath}, + sync::{self, commit_files::OldNew, CommitId, RepoPath}, AsyncGitNotification, StatusItem, }; use crossbeam_channel::Sender; @@ -36,6 +36,15 @@ impl From<(CommitId, CommitId)> for CommitFilesParams { } } +impl From> for CommitFilesParams { + fn from(old_new: OldNew) -> Self { + Self { + id: old_new.new, + other: Some(old_new.old), + } + } +} + /// pub struct AsyncCommitFiles { current: diff --git a/asyncgit/src/diff.rs b/asyncgit/src/diff.rs index 20de8f5614..03a04398ba 100644 --- a/asyncgit/src/diff.rs +++ b/asyncgit/src/diff.rs @@ -1,7 +1,10 @@ use crate::{ error::Result, hash, - sync::{self, diff::DiffOptions, CommitId, RepoPath}, + sync::{ + self, commit_files::OldNew, diff::DiffOptions, CommitId, + RepoPath, + }, AsyncGitNotification, FileDiff, }; use crossbeam_channel::Sender; @@ -17,7 +20,7 @@ use std::{ #[derive(Debug, Hash, Clone, PartialEq, Eq)] pub enum DiffType { /// diff two commits - Commits((CommitId, CommitId)), + Commits(OldNew), /// diff in a given commit Commit(CommitId), /// diff against staged file diff --git a/asyncgit/src/sync/commit_files.rs b/asyncgit/src/sync/commit_files.rs index 7702670af7..b9b545a8b6 100644 --- a/asyncgit/src/sync/commit_files.rs +++ b/asyncgit/src/sync/commit_files.rs @@ -8,7 +8,36 @@ use crate::{ }; use git2::{Diff, Repository}; use scopetime::scope_time; -use std::cmp::Ordering; + +/// struct containing a new and an old version +#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] +pub struct OldNew { + /// The old version + pub old: T, + /// The new version + pub new: T, +} + +/// Sort two commits. +pub fn sort_commits( + repo: &Repository, + commits: (CommitId, CommitId), +) -> Result> { + if repo.graph_descendant_of( + commits.0.get_oid(), + commits.1.get_oid(), + )? { + Ok(OldNew { + old: commits.1, + new: commits.0, + }) + } else { + Ok(OldNew { + old: commits.0, + new: commits.1, + }) + } +} /// get all files that are part of a commit pub fn get_commit_files( @@ -21,7 +50,12 @@ pub fn get_commit_files( let repo = repo(repo_path)?; let diff = if let Some(other) = other { - get_compare_commits_diff(&repo, (id, other), None, None)? + get_compare_commits_diff( + &repo, + sort_commits(&repo, (id, other))?, + None, + None, + )? } else { get_commit_diff(repo_path, &repo, id, None, None)? }; @@ -49,26 +83,20 @@ pub fn get_commit_files( #[allow(clippy::needless_pass_by_value)] pub fn get_compare_commits_diff( repo: &Repository, - ids: (CommitId, CommitId), + ids: OldNew, pathspec: Option, options: Option, ) -> Result> { // scope_time!("get_compare_commits_diff"); - - let commits = ( - repo.find_commit(ids.0.into())?, - repo.find_commit(ids.1.into())?, - ); - - let commits = if commits.0.time().cmp(&commits.1.time()) - == Ordering::Greater - { - (commits.1, commits.0) - } else { - commits + let commits = OldNew { + old: repo.find_commit(ids.old.into())?, + new: repo.find_commit(ids.new.into())?, }; - let trees = (commits.0.tree()?, commits.1.tree()?); + let trees = OldNew { + old: commits.old.tree()?, + new: commits.new.tree()?, + }; let mut opts = git2::DiffOptions::new(); if let Some(options) = options { @@ -81,9 +109,9 @@ pub fn get_compare_commits_diff( } opts.show_binary(true); - let diff = repo.diff_tree_to_tree( - Some(&trees.0), - Some(&trees.1), + let diff: Diff<'_> = repo.diff_tree_to_tree( + Some(&trees.old), + Some(&trees.new), Some(&mut opts), )?; diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index 77bd5886ea..ea8a755e55 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -1,7 +1,9 @@ //! sync git api for fetching a diff use super::{ - commit_files::{get_commit_diff, get_compare_commits_diff}, + commit_files::{ + get_commit_diff, get_compare_commits_diff, OldNew, + }, utils::{get_head_repo, work_dir}, CommitId, RepoPath, }; @@ -232,7 +234,7 @@ pub fn get_diff_commit( /// get file changes of a diff between two commits pub fn get_diff_commits( repo_path: &RepoPath, - ids: (CommitId, CommitId), + ids: OldNew, p: String, options: Option, ) -> Result { @@ -240,12 +242,8 @@ pub fn get_diff_commits( let repo = repo(repo_path)?; let work_dir = work_dir(&repo)?; - let diff = get_compare_commits_diff( - &repo, - (ids.0, ids.1), - Some(p), - options, - )?; + let diff = + get_compare_commits_diff(&repo, ids, Some(p), options)?; raw_diff_to_file_diff(&diff, work_dir) } diff --git a/src/components/commit_details/compare_details.rs b/src/components/commit_details/compare_details.rs index 015e3ff37f..b52869cdc5 100644 --- a/src/components/commit_details/compare_details.rs +++ b/src/components/commit_details/compare_details.rs @@ -12,7 +12,9 @@ use crate::{ ui::style::SharedTheme, }; use anyhow::Result; -use asyncgit::sync::{self, CommitDetails, CommitId, RepoPathRef}; +use asyncgit::sync::{ + self, commit_files::OldNew, CommitDetails, CommitId, RepoPathRef, +}; use crossterm::event::Event; use ratatui::{ backend::Backend, @@ -23,7 +25,7 @@ use ratatui::{ pub struct CompareDetailsComponent { repo: RepoPathRef, - data: Option<(CommitDetails, CommitDetails)>, + data: Option>, theme: SharedTheme, focused: bool, } @@ -43,24 +45,20 @@ impl CompareDetailsComponent { } } - pub fn set_commits(&mut self, ids: Option<(CommitId, CommitId)>) { + pub fn set_commits(&mut self, ids: Option>) { self.data = ids.and_then(|ids| { - let c1 = - sync::get_commit_details(&self.repo.borrow(), ids.0) - .ok(); - let c2 = - sync::get_commit_details(&self.repo.borrow(), ids.1) - .ok(); - - c1.and_then(|c1| { - c2.map(|c2| { - if c1.author.time < c2.author.time { - (c1, c2) - } else { - (c2, c1) - } - }) - }) + let old = sync::get_commit_details( + &self.repo.borrow(), + ids.old, + ) + .ok()?; + let new = sync::get_commit_details( + &self.repo.borrow(), + ids.new, + ) + .ok()?; + + Some(OldNew { old, new }) }); } @@ -125,9 +123,9 @@ impl DrawableComponent for CompareDetailsComponent { dialog_paragraph( &strings::commit::compare_details_info_title( true, - data.0.short_hash(), + data.old.short_hash(), ), - Text::from(self.get_commit_text(&data.0)), + Text::from(self.get_commit_text(&data.old)), &self.theme, false, ), @@ -138,9 +136,9 @@ impl DrawableComponent for CompareDetailsComponent { dialog_paragraph( &strings::commit::compare_details_info_title( false, - data.1.short_hash(), + data.new.short_hash(), ), - Text::from(self.get_commit_text(&data.1)), + Text::from(self.get_commit_text(&data.new)), &self.theme, false, ), diff --git a/src/components/commit_details/mod.rs b/src/components/commit_details/mod.rs index 35317670e5..622fccaf98 100644 --- a/src/components/commit_details/mod.rs +++ b/src/components/commit_details/mod.rs @@ -15,7 +15,7 @@ use crate::{ }; use anyhow::Result; use asyncgit::{ - sync::{CommitTags, RepoPathRef}, + sync::{commit_files::OldNew, CommitTags, RepoPathRef}, AsyncCommitFiles, AsyncGitNotification, CommitFilesParams, }; use compare_details::CompareDetailsComponent; @@ -105,8 +105,10 @@ impl CommitDetailsComponent { self.file_tree.set_commit(Some(id.id)); if let Some(other) = id.other { - self.compare_details - .set_commits(Some((id.id, other))); + self.compare_details.set_commits(Some(OldNew { + new: id.id, + old: other, + })); } else { self.single_details .set_commit(Some(id.id), tags.clone()); diff --git a/src/components/compare_commits.rs b/src/components/compare_commits.rs index bd92750a30..a5832d07f2 100644 --- a/src/components/compare_commits.rs +++ b/src/components/compare_commits.rs @@ -13,7 +13,7 @@ use crate::{ }; use anyhow::Result; use asyncgit::{ - sync::{self, CommitId, RepoPathRef}, + sync::{self, commit_files::OldNew, CommitId, RepoPathRef}, AsyncDiff, AsyncGitNotification, CommitFilesParams, DiffParams, DiffType, }; @@ -239,16 +239,19 @@ impl CompareCommitsComponent { Ok(()) } - fn get_ids(&self) -> Option<(CommitId, CommitId)> { + fn get_ids(&self) -> Option> { let other = self .open_request .as_ref() .and_then(|open| open.compare_id); - self.open_request - .as_ref() - .map(|open| open.commit_id) - .zip(other) + let this = + self.open_request.as_ref().map(|open| open.commit_id); + + Some(OldNew { + old: other?, + new: this?, + }) } /// called when any tree component changed selection From fe8e58a184584115db1ef3c7bfc89a2a0927b287 Mon Sep 17 00:00:00 2001 From: Joshix Date: Sun, 20 Aug 2023 18:00:00 +0000 Subject: [PATCH 2/4] add entry to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d98691b086..8822cc15a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * fix expansion of `~` in `commit.template` ([#1745](https://github.com/extrawurst/gitui/pull/1745)) * fix hunk (un)staging/reset for # of context lines != 3 ([#1746](https://github.com/extrawurst/gitui/issues/1746)) * fix delay when opening external editor ([#1506](https://github.com/extrawurst/gitui/issues/1506)) +* fix ordering of commits in diff view [@Joshix-1](https://github.com/Joshix-1) ([#1747](https://github.com/extrawurst/gitui/issues/1747)) ## [0.23.0] - 2022-06-19 From 3b302676a1d21f28b967518d7b894eddf61edb6f Mon Sep 17 00:00:00 2001 From: Joshix Date: Sun, 3 Sep 2023 18:00:00 +0000 Subject: [PATCH 3/4] brackets --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8822cc15a7..18e3f71429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * fix expansion of `~` in `commit.template` ([#1745](https://github.com/extrawurst/gitui/pull/1745)) * fix hunk (un)staging/reset for # of context lines != 3 ([#1746](https://github.com/extrawurst/gitui/issues/1746)) * fix delay when opening external editor ([#1506](https://github.com/extrawurst/gitui/issues/1506)) -* fix ordering of commits in diff view [@Joshix-1](https://github.com/Joshix-1) ([#1747](https://github.com/extrawurst/gitui/issues/1747)) +* fix ordering of commits in diff view [[@Joshix-1](https://github.com/Joshix-1)]([#1747](https://github.com/extrawurst/gitui/issues/1747)) ## [0.23.0] - 2022-06-19 From 393b5c1fa5ca55c2b40c2d8cbc01e3ab46ddb952 Mon Sep 17 00:00:00 2001 From: Joshix Date: Mon, 12 Feb 2024 17:46:01 +0100 Subject: [PATCH 4/4] remove unused imports --- src/components/commit_details/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/commit_details/mod.rs b/src/components/commit_details/mod.rs index 22dc919fe3..e9cfd6b97c 100644 --- a/src/components/commit_details/mod.rs +++ b/src/components/commit_details/mod.rs @@ -14,8 +14,8 @@ use crate::{ }; use anyhow::Result; use asyncgit::{ - sync::{commit_files::OldNew, CommitTags, RepoPathRef}, - AsyncCommitFiles, AsyncGitNotification, CommitFilesParams, + sync::{commit_files::OldNew, CommitTags}, + AsyncCommitFiles, CommitFilesParams, }; use compare_details::CompareDetailsComponent; use crossterm::event::Event;