Repository: orhun/git-cliff
Pull request #1121 (2025-03-26 10:27:09)

feat(git): disable sorting commits topologically (#804)
## Description

Add sorting option for commits 

## Motivation and Context

This change is related to #804 
When I tried git-cliff on my personal git repository, the commits order was not correct. 
I finally found why the order was not correct. The CHANGELOD generated got the same order than the command git log --oneline --topo-order.


## How Has This Been Tested?

I did not add a specific test for this feature, I apologize, I just installed rust for the first time yesterday and I never used GitHub before ..
I tried to follow your recommandation.
I launched:
- cargo build
- cargo test
- cargo fmt
- cargo clippy

I also tested on my side that the sort is functional on my repro. I tested the command line option --disable-topo-order-commits
I also tested this option inside my file cliff.toml

## Screenshots / Logs (if applicable)

## Types of Changes


- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Documentation (no code change)
- [ ] Refactor (refactoring production code)
- [ ] Other <!--- (provide information) -->

## Checklist:

- [X] My code follows the code style of this project.
I launched cargo fmt and cargo clippy
- [X] I have updated the documentation accordingly.
Maybe the text can be improved
- [X] I have formatted the code with [rustfmt](https://github.com/rust-lang/rustfmt).
- [X] I checked the lints with [clippy](https://github.com/rust-lang/rust-clippy).
- [ ] I have added tests to cover my changes.
- [X] All new and existing tests passed.

<!--- Thank you for contributing to git-cliff! ⛰️  -->


welcome[bot]: Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/orhun/git-cliff/pull/1121?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) Report
Attention: Patch coverage is `50.00000%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 41.22%. Comparing base [(`cdce20f`)](https://app.codecov.io/gh/orhun/git-cliff/commit/cdce20f39104b797c17a85aa5988c608f3437e11?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) to head [(`553b70a`)](https://app.codecov.io/gh/orhun/git-cliff/commit/553b70a3771e773cfaf9e03e52c5a17485a113a3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z).

| [Files with missing lines](https://app.codecov.io/gh/orhun/git-cliff/pull/1121?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) | Patch % | Lines |
|---|---|---|
| [git-cliff-core/src/repo.rs](https://app.codecov.io/gh/orhun/git-cliff/pull/1121?src=pr&el=tree&filepath=git-cliff-core%2Fsrc%2Frepo.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z#diff-Z2l0LWNsaWZmLWNvcmUvc3JjL3JlcG8ucnM=) | 66.67% | [1 Missing :warning: ](https://app.codecov.io/gh/orhun/git-cliff/pull/1121?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) |
| [git-cliff/src/lib.rs](https://app.codecov.io/gh/orhun/git-cliff/pull/1121?src=pr&el=tree&filepath=git-cliff%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z#diff-Z2l0LWNsaWZmL3NyYy9saWIucnM=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/orhun/git-cliff/pull/1121?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1121      +/-   ##
==========================================
- Coverage   41.27%   41.22%   -0.04%     
==========================================
  Files          21       21              
  Lines        1820     1822       +2     
==========================================
  Hits          751      751              
- Misses       1069     1071       +2     
```

| [Flag](https://app.codecov.io/gh/orhun/git-cliff/pull/1121/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) | Coverage Δ | |
|---|---|---|
| [unit-tests](https://app.codecov.io/gh/orhun/git-cliff/pull/1121/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z) | `41.22% <50.00%> (-0.04%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/orhun/git-cliff/pull/1121?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Orhun+Parmaks%C4%B1z).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
Cyclonit: I would propose refactoring the system to have two options:

- `git.commit_order_by` = `topology`/`time`
- `changelog.release_order_by` = `topology`/`time`

Having enums instead of booleans improves readability significantly. But I know @orhun likes using as few words as possible ;)

However, we do not have any way of soft deprecating old config options and adding new ones beside it. That is something that would have to be designed.
SebClapie: Hello,

Thanks @orhun for your feedback.
I will a look to the fixture, and I may come back to you if I need help

I can rename `disable_topo_order_commits `to `topo_order_commits`.
Or do I look for @Cyclonit proposal  ?

- `git.commit_order_by = topology/time`


orhun: > I would propose refactoring the system to have two options:
> 
>     * `git.commit_order_by` = `topology`/`time`
> 
>     * `changelog.release_order_by` = `topology`/`time`

In that case we also need to deprecate the old options. I'd prefer we go with `topo_order_commits` for now and revamp the configuration later.

> However, we do not have any way of soft deprecating old config options and adding new ones beside it. That is something that would have to be designed.

We can probably add a warning while parsing the config 🤔 I'll look into it.

LMK @SebClapie when this is ready for review.


SebClapie: Hello @orhun ,

I updated the code to rename the option `topo_order_commits `, and the default value is true to keep previous behavior
I added a new test fixture, but I don't know why I got this issue "line 2: ./commit.sh: Permission denied". I follow your procedure for add a new ficture, and it works locally for me.
Cyclonit: > Hello @orhun ,
> 
> ... I added a new test fixture, but I don't know why I got this issue "line 2: ./commit.sh: Permission denied".

The `commit.sh` you creqted does not have POSIX permissions assogned to it. This usually happens because you created the file on windows. Run this to fix it, commit and push again:

```sh
git update-index --chmod=+x <file>
```
SebClapie: The Typos action failed. But I don't think it's linked to my modification
I don't know how to retrigger it ....
Cyclonit: > > Some minor things left :)
> > 
> > * The fixture config can be simplified.
> > * A small tweak in docs is needed (see my comment)
> > * We need an issue to track the deprecation of config
> 
> Hello @orhun what do you mean about the deprecation of config ? You want to replace `topo_order` by `topo_order_tags `?

The current config schema has grown historically and is in need of a refactor. I attempted to do this back in #541, but couldn't keep up with other changes at the time. The goal of improving the config in general remains though.

One of the requirements for overhauling the config is the ability to soft deprecate old options. Otherwise any change to the config would be an immediate breaking change.

I don't think you need to care about that right now.

Patch:
diff --git a/config/cliff.toml b/config/cliff.toml
index b95f080..f023a1e 100644
--- a/config/cliff.toml
+++ b/config/cliff.toml
@@ -94,6 +94,8 @@ link_parsers = []
 use_branch_tags = false
 # Order releases topologically instead of chronologically.
 topo_order = false
+# Order releases topologically instead of chronologically.
+topo_order_commits = true
 # Order of commits in each group/release within the changelog.
 # Allowed values: newest, oldest
 sort_commits = "oldest"
diff --git a/git-cliff-core/src/changelog.rs b/git-cliff-core/src/changelog.rs
index 6bfb901..d696e04 100644
--- a/git-cliff-core/src/changelog.rs
+++ b/git-cliff-core/src/changelog.rs
@@ -862,6 +862,7 @@ mod test {
 				count_tags:               None,
 				use_branch_tags:          false,
 				topo_order:               false,
+				topo_order_commits:       true,
 				sort_commits:             String::from("oldest"),
 				link_parsers:             [].to_vec(),
 				limit_commits:            None,
diff --git a/git-cliff-core/src/config.rs b/git-cliff-core/src/config.rs
index 724e986..38d6a09 100644
--- a/git-cliff-core/src/config.rs
+++ b/git-cliff-core/src/config.rs
@@ -130,6 +130,8 @@ pub struct GitConfig {
 	pub use_branch_tags:          bool,
 	/// Order releases topologically instead of chronologically.
 	pub topo_order:               bool,
+	/// Order commits chronologically instead of topologically.
+	pub topo_order_commits:       bool,
 	/// How to order commits in each group/release within the changelog.
 	pub sort_commits:             String,
 	/// Limit the total number of commits included in the changelog.
diff --git a/git-cliff-core/src/repo.rs b/git-cliff-core/src/repo.rs
index 47e5888..21b534e 100644
--- a/git-cliff-core/src/repo.rs
+++ b/git-cliff-core/src/repo.rs
@@ -118,9 +118,15 @@ impl Repository {
 		range: Option<&str>,
 		include_path: Option<Vec<Pattern>>,
 		exclude_path: Option<Vec<Pattern>>,
+		topo_order_commits: bool,
 	) -> Result<Vec<Commit>> {
 		let mut revwalk = self.inner.revwalk()?;
-		revwalk.set_sorting(Sort::TOPOLOGICAL)?;
+		if topo_order_commits {
+			revwalk.set_sorting(Sort::TOPOLOGICAL)?;
+		} else {
+			revwalk.set_sorting(Sort::TIME)?;
+		}
+
 		Self::set_commit_range(&mut revwalk, range).map_err(|e| {
 			Error::SetCommitRangeError(
 				range.map(String::from).unwrap_or_else(|| "?".to_string()),
@@ -621,7 +627,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, None, None, false)?;
 		let last_commit =
 			AppCommit::from(&commits.first().expect("no commits found").clone());
 		assert_eq!(get_last_commit_hash()?, last_commit.id);
@@ -739,7 +745,7 @@ mod test {
 		let repository = get_repository()?;
 		// a close descendant of the root commit
 		let range = Some("eea3914c7ab07472841aa85c36d11bdb2589a234");
-		let commits = repository.commits(range, None, None)?;
+		let commits = repository.commits(range, None, None, false)?;
 		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 ceff7d7..a7bc0e6 100644
--- a/git-cliff/src/lib.rs
+++ b/git-cliff/src/lib.rs
@@ -173,7 +173,12 @@ fn process_repository<'a>(
 		}
 	} else if args.latest || args.current {
 		if tags.len() < 2 {
-			let commits = repository.commits(None, None, None)?;
+			let commits = repository.commits(
+				None,
+				None,
+				None,
+				config.git.topo_order_commits,
+			)?;
 			if let (Some(tag1), Some(tag2)) = (
 				commits.last().map(|c| c.id().to_string()),
 				tags.get_index(0).map(|(k, _)| k),
@@ -241,6 +246,7 @@ fn process_repository<'a>(
 		commit_range.as_deref(),
 		include_path,
 		args.exclude_path.clone(),
+		config.git.topo_order_commits,
 	)?;
 	if let Some(commit_limit_value) = config.git.limit_commits {
 		commits.truncate(commit_limit_value);
diff --git a/website/docs/configuration/git.md b/website/docs/configuration/git.md
index 1fa0317..69f311f 100644
--- a/website/docs/configuration/git.md
+++ b/website/docs/configuration/git.md
@@ -24,6 +24,7 @@ tag_pattern = "v[0-9].*"
 skip_tags = "v0.1.0-beta.1"
 ignore_tags = ""
 topo_order = false
+topo_order_commits = true
 sort_commits = "oldest"
 link_parsers = [
     { pattern = "#(\\d+)", href = "https://github.com/orhun/git-cliff/issues/$1"},
@@ -252,6 +253,16 @@ If set to `true`, tags are processed in topological order instead of chronologic
 
 This can also be achieved by using the `--topo-order` command line flag.
 
+### topo_order_commits
+
+If set to `true`, commits are processed in topological order instead of chronological.
+
+```toml
+# if false, sorting commit is equivalent to git log 
+# if true (default), sorting commit is equivalent to git log --topo-order
+topo_order_commits = false
+```
+
 ### sort_commits
 
 Sort the commits inside sections by specified order.


Test patch:
diff --git a/.github/fixtures/test-topo-order-commits/cliff.toml b/.github/fixtures/test-topo-order-commits/cliff.toml
new file mode 100644
index 00000000..b8139b1
--- /dev/null
+++ b/.github/fixtures/test-topo-order-commits/cliff.toml
@@ -0,0 +1,7 @@
+# git-cliff ~ configuration file
+# https://git-cliff.org/docs/configuration
+
+
+[git]
+
+topo_order_commits = false
\ No newline at end of file
diff --git a/.github/fixtures/test-topo-order-commits/commit.sh b/.github/fixtures/test-topo-order-commits/commit.sh
new file mode 100755
index 00000000..f4b6c67
--- /dev/null
+++ b/.github/fixtures/test-topo-order-commits/commit.sh
@@ -0,0 +1,16 @@
+#!/usr/bin/env bash
+set -e
+
+GIT_COMMITTER_DATE="2022-04-06 03:25:08" git commit --allow-empty -m "Initial commit"
+GIT_COMMITTER_DATE="2022-04-06 03:25:09" git commit --allow-empty -m "feat: add feature 1"
+GIT_COMMITTER_DATE="2022-04-06 03:25:10" git commit --allow-empty -m "fix: fix feature 1"
+git tag v0.1.0
+GIT_COMMITTER_DATE="2022-04-06 03:25:11" git commit --allow-empty -m "feat(gui): add feature 2"
+GIT_COMMITTER_DATE="2022-04-06 03:25:12" git commit --allow-empty -m "fix(gui): fix feature 2"
+git tag v0.2.0
+GIT_COMMITTER_DATE="2022-04-06 03:25:25" git commit --allow-empty -m "test: add tests"
+GIT_COMMITTER_DATE="2022-04-06 03:25:25" git checkout v0.1.0
+GIT_COMMITTER_DATE="2022-04-06 03:25:13" git commit --allow-empty -m "fix: fix again feature 1"
+git tag v0.1.1
+git checkout master
+git merge v0.1.1 --no-edit
diff --git a/.github/fixtures/test-topo-order-commits/expected.md b/.github/fixtures/test-topo-order-commits/expected.md
new file mode 100644
index 00000000..124385f
--- /dev/null
+++ b/.github/fixtures/test-topo-order-commits/expected.md
@@ -0,0 +1,37 @@
+# Changelog
+
+All notable changes to this project will be documented in this file.
+
+## [unreleased]
+
+### 🧪 Testing
+
+- Add tests
+
+## [0.1.1] - 2022-04-06
+
+### 🐛 Bug Fixes
+
+- Fix again feature 1
+
+## [0.2.0] - 2022-04-06
+
+### 🚀 Features
+
+- *(gui)* Add feature 2
+
+### 🐛 Bug Fixes
+
+- *(gui)* Fix feature 2
+
+## [0.1.0] - 2022-04-06
+
+### 🚀 Features
+
+- Add feature 1
+
+### 🐛 Bug Fixes
+
+- Fix feature 1
+
+<!-- generated by git-cliff -->
diff --git a/.github/workflows/test-fixtures.yml b/.github/workflows/test-fixtures.yml
index 567fba3..6b33d94 100644
--- a/.github/workflows/test-fixtures.yml
+++ b/.github/workflows/test-fixtures.yml
@@ -110,6 +110,7 @@ jobs:
           - fixtures-name: test-require-conventional-skipped
           - fixtures-name: test-remote-config
             command: --config-url "https://github.com/orhun/git-cliff/blob/main/.github/fixtures/new-fixture-template/cliff.toml?raw=true"
+          - fixtures-name: test-topo-order-commits
 
     steps:
       - name: Checkout
diff --git a/git-cliff-core/tests/integration_test.rs b/git-cliff-core/tests/integration_test.rs
index eb9dfee..fd04722 100644
--- a/git-cliff-core/tests/integration_test.rs
+++ b/git-cliff-core/tests/integration_test.rs
@@ -124,6 +124,7 @@ fn generate_changelog() -> Result<()> {
 		count_tags:               None,
 		use_branch_tags:          false,
 		topo_order:               false,
+		topo_order_commits:       true,
 		sort_commits:             String::from("oldest"),
 		link_parsers:             vec![
 			LinkParser {