From bba70d9d4d792e5b0404c729e9da0a9ea224c7fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rare=C8=99=20Cosma?= Date: Wed, 2 Oct 2024 17:02:29 +0300 Subject: [PATCH 1/5] feat(git): latest with one tag should include root --- git-cliff-core/src/repo.rs | 12 ++++++++++-- git-cliff/src/lib.rs | 5 ++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/git-cliff-core/src/repo.rs b/git-cliff-core/src/repo.rs index 8fe1f08731..35bd1403bd 100644 --- a/git-cliff-core/src/repo.rs +++ b/git-cliff-core/src/repo.rs @@ -72,13 +72,21 @@ impl Repository { pub fn commits( &self, range: Option<&str>, + include_root: bool, include_path: Option>, exclude_path: Option>, ) -> Result> { let mut revwalk = self.inner.revwalk()?; revwalk.set_sorting(Sort::TOPOLOGICAL)?; if let Some(range) = range { - revwalk.push_range(range)?; + // Push only the last range commit if we need to start at root. + if include_root { + if let Some(dots_index) = range.find("..") { + revwalk.push(Oid::from_str(&range[dots_index + 2..])?)?; + } + } else { + revwalk.push_range(range)?; + } } else { revwalk.push_head()?; } @@ -488,7 +496,7 @@ mod test { #[test] fn get_latest_commit() -> Result<()> { let repository = get_repository()?; - let commits = repository.commits(None, None, None)?; + let commits = repository.commits(None, false, None, None)?; let last_commit = AppCommit::from(&commits.first().expect("no commits found").clone()); assert_eq!(get_last_commit_hash()?, last_commit.id); diff --git a/git-cliff/src/lib.rs b/git-cliff/src/lib.rs index b2c29f95d9..ff72c343ae 100644 --- a/git-cliff/src/lib.rs +++ b/git-cliff/src/lib.rs @@ -160,18 +160,20 @@ fn process_repository<'a>( // Parse commits. let mut commit_range = args.range.clone(); + let mut include_root = false; if args.unreleased { if let Some(last_tag) = tags.last().map(|(k, _)| k) { commit_range = Some(format!("{last_tag}..HEAD")); } } else if args.latest || args.current { if tags.len() < 2 { - let commits = repository.commits(None, None, None)?; + let commits = repository.commits(None, false, None, None)?; if let (Some(tag1), Some(tag2)) = ( commits.last().map(|c| c.id().to_string()), tags.get_index(0).map(|(k, _)| k), ) { commit_range = Some(format!("{tag1}..{tag2}")); + include_root = true; } } else { let mut tag_index = tags.len() - 2; @@ -208,6 +210,7 @@ fn process_repository<'a>( } let mut commits = repository.commits( commit_range.as_deref(), + include_root, args.include_path.clone(), args.exclude_path.clone(), )?; From 6ec995c39e135448998a2a455335b15baed96d0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rare=C8=99=20Cosma?= Date: Wed, 2 Oct 2024 17:03:33 +0300 Subject: [PATCH 2/5] feat(git): test root commit with one tag --- .../test-latest-with-one-tag/commit.sh | 2 +- .../test-latest-with-one-tag/expected.md | 1 + git-cliff-core/src/repo.rs | 22 +++++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/fixtures/test-latest-with-one-tag/commit.sh b/.github/fixtures/test-latest-with-one-tag/commit.sh index 6c5c90caaf..361943d085 100755 --- a/.github/fixtures/test-latest-with-one-tag/commit.sh +++ b/.github/fixtures/test-latest-with-one-tag/commit.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash set -e -GIT_COMMITTER_DATE="2021-01-23 01:23:45" git commit --allow-empty -m "Initial commit" +GIT_COMMITTER_DATE="2021-01-23 01:23:45" git commit --allow-empty -m "feat: initial commit" GIT_COMMITTER_DATE="2021-01-23 01:23:46" git commit --allow-empty -m "feat: add feature 1" git tag v0.1.0 diff --git a/.github/fixtures/test-latest-with-one-tag/expected.md b/.github/fixtures/test-latest-with-one-tag/expected.md index 1b15d95c46..e7fdd52b94 100644 --- a/.github/fixtures/test-latest-with-one-tag/expected.md +++ b/.github/fixtures/test-latest-with-one-tag/expected.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. ### Feat +- Initial commit - Add feature 1 diff --git a/git-cliff-core/src/repo.rs b/git-cliff-core/src/repo.rs index 35bd1403bd..a666a504b6 100644 --- a/git-cliff-core/src/repo.rs +++ b/git-cliff-core/src/repo.rs @@ -472,6 +472,18 @@ mod test { .to_string()) } + fn get_root_commit_hash() -> Result { + Ok(str::from_utf8( + Command::new("git") + .args(["rev-list", "--max-parents=0", "HEAD"]) + .output()? + .stdout + .as_ref(), + )? + .trim_ascii_end() + .to_string()) + } + fn get_last_tag() -> Result { Ok(str::from_utf8( Command::new("git") @@ -594,6 +606,16 @@ mod test { Ok(()) } + #[test] + fn includes_root_commit() -> Result<()> { + let repository = get_repository()?; + let commits = repository.commits(None, true, None, None)?; + let root_commit = + AppCommit::from(&commits.last().expect("no commits found").clone()); + assert_eq!(get_root_commit_hash()?, root_commit.id); + Ok(()) + } + fn create_temp_repo() -> (Repository, TempDir) { let temp_dir = TempDir::with_prefix("git-cliff-").expect("failed to create temp dir"); From 06f96c279f67987a1d2e2323d1a4eec1e22d8df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rare=C8=99=20Cosma?= Date: Sat, 5 Oct 2024 11:19:55 +0300 Subject: [PATCH 3/5] feat(git): remove the include_root boolean flag Use the fact that a range contains (or doesn't) contain ".." as a discriminant between the two cases: - ".." means full (left-exclusive) range between two commits; - no ".." means everything from the root commit (inclusive) to the commit sha in the range --- git-cliff-core/src/repo.rs | 17 ++++++++--------- git-cliff/src/lib.rs | 11 ++++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/git-cliff-core/src/repo.rs b/git-cliff-core/src/repo.rs index a666a504b6..4ae08ccd91 100644 --- a/git-cliff-core/src/repo.rs +++ b/git-cliff-core/src/repo.rs @@ -72,20 +72,17 @@ impl Repository { pub fn commits( &self, range: Option<&str>, - include_root: bool, include_path: Option>, exclude_path: Option>, ) -> Result> { let mut revwalk = self.inner.revwalk()?; revwalk.set_sorting(Sort::TOPOLOGICAL)?; if let Some(range) = range { - // Push only the last range commit if we need to start at root. - if include_root { - if let Some(dots_index) = range.find("..") { - revwalk.push(Oid::from_str(&range[dots_index + 2..])?)?; - } - } else { + if range.contains("..") { revwalk.push_range(range)?; + } else { + // when we get a single sha as our "range" => start at root. + revwalk.push(Oid::from_str(&range)?)?; } } else { revwalk.push_head()?; @@ -508,7 +505,7 @@ mod test { #[test] fn get_latest_commit() -> Result<()> { let repository = get_repository()?; - let commits = repository.commits(None, false, None, None)?; + let commits = repository.commits(None, None, None)?; let last_commit = AppCommit::from(&commits.first().expect("no commits found").clone()); assert_eq!(get_last_commit_hash()?, last_commit.id); @@ -609,7 +606,9 @@ mod test { #[test] fn includes_root_commit() -> Result<()> { let repository = get_repository()?; - let commits = repository.commits(None, true, None, None)?; + // a close descendant of the root commit + let range = Some("eea3914c7ab07472841aa85c36d11bdb2589a234"); + let commits = repository.commits(range, None, None)?; let root_commit = AppCommit::from(&commits.last().expect("no commits found").clone()); assert_eq!(get_root_commit_hash()?, root_commit.id); diff --git a/git-cliff/src/lib.rs b/git-cliff/src/lib.rs index ff72c343ae..95d37b738e 100644 --- a/git-cliff/src/lib.rs +++ b/git-cliff/src/lib.rs @@ -160,20 +160,22 @@ fn process_repository<'a>( // Parse commits. let mut commit_range = args.range.clone(); - let mut include_root = false; if args.unreleased { if let Some(last_tag) = tags.last().map(|(k, _)| k) { commit_range = Some(format!("{last_tag}..HEAD")); } } else if args.latest || args.current { if tags.len() < 2 { - let commits = repository.commits(None, false, None, None)?; + let commits = repository.commits(None, None, None)?; if let (Some(tag1), Some(tag2)) = ( commits.last().map(|c| c.id().to_string()), tags.get_index(0).map(|(k, _)| k), ) { - commit_range = Some(format!("{tag1}..{tag2}")); - include_root = true; + if tags.len() == 1 { + commit_range = Some(tag2.to_owned()); + } else { + commit_range = Some(format!("{tag1}..{tag2}")); + } } } else { let mut tag_index = tags.len() - 2; @@ -210,7 +212,6 @@ fn process_repository<'a>( } let mut commits = repository.commits( commit_range.as_deref(), - include_root, args.include_path.clone(), args.exclude_path.clone(), )?; From 90e0202bcc69984a047904ccb41e28724c003da2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rare=C8=99=20Cosma?= Date: Sat, 5 Oct 2024 11:28:04 +0300 Subject: [PATCH 4/5] fix: remove unnecessary reference --- git-cliff-core/src/repo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-cliff-core/src/repo.rs b/git-cliff-core/src/repo.rs index 4ae08ccd91..4d56b6efa4 100644 --- a/git-cliff-core/src/repo.rs +++ b/git-cliff-core/src/repo.rs @@ -82,7 +82,7 @@ impl Repository { revwalk.push_range(range)?; } else { // when we get a single sha as our "range" => start at root. - revwalk.push(Oid::from_str(&range)?)?; + revwalk.push(Oid::from_str(range)?)?; } } else { revwalk.push_head()?; From b5ecc35ffc63e01e753ef443b4c5a7517c7acf90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rare=C8=99=20Cosma?= Date: Sat, 5 Oct 2024 11:49:23 +0300 Subject: [PATCH 5/5] nit: gentler English in comment --- git-cliff-core/src/repo.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-cliff-core/src/repo.rs b/git-cliff-core/src/repo.rs index 4d56b6efa4..143809831f 100644 --- a/git-cliff-core/src/repo.rs +++ b/git-cliff-core/src/repo.rs @@ -81,7 +81,7 @@ impl Repository { if range.contains("..") { revwalk.push_range(range)?; } else { - // when we get a single sha as our "range" => start at root. + // When a single SHA is provided as the "range", start from the root. revwalk.push(Oid::from_str(range)?)?; } } else {