Add commit_range to template context

[jdrst]

Hey,

i would like to add links to diffs of certain files to the changelog.
Is there a way to get the commit range git cliff uses or was called with (in my case git cliff commitA..commitB)? As far as i could see it's not part of the context.

Thanks

[welcome]

Thanks for opening your first issue at git-cliff! Be sure to follow the issue template! ⛰️

[orhun]

Good idea! We simply need to add commit_range to Release struct and update the other parts accordingly. See #721 for an example.

Marking this as good-first-issue, would you be interested in contributing?

[jdrst]

i'll try to make some time next week and have a look.

[orhun]

Perfect, assigned to you! Let me know if you need any assistance.

[jdrst]

Hey,

i finally could make the time to have a look. Unless i'm overlooking something this might not be as straight forward as thought. Here are my observations:

Currently the commit range is passed when getting the commits for changelog creation, we don't have ownership of the repo there.
As far as i could tell, we might not even "know" the actual commits used, at that time, because they might be internal to the revwalker. What we (afterwards) do have, is a list of sorted commits that is produced. So it might be possible to add something here akin to

//...
	let first_commit = commits.first().unwrap().id();
	let last_commit = commits.last().unwrap().id();
	let commit_range =
		CommitRange::new(&first_commit.to_string(), &last_commit.to_string());
	for git_commit in commits.iter().rev() {
		let release = releases.last_mut().unwrap();
                //..
		release.commit_range = Some(commit_range.clone());
// ...

with

/// Commit range (from..to)
pub struct CommitRange {
	from: String,
	to:   String,
}

From what i could tell this is currently not easily testable though, right?

Let me know your thoughts.

[Cyclonit]

Can you elaborate on the use case for this request? Do you want to generate a diff between a change's commit and the previous release?

[orhun]

Do you want to generate a diff between a change's commit and the previous release?

I think yes... I cannot think of another use case.

As far as i could tell, we might not even "know" the actual commits used, at that time, because they might be internal to the revwalker.

Hmm yeah, good point.

let commit_range =
CommitRange::new(&first_commit.to_string(), &last_commit.to_string());

Yup, that looks good to me. I think that would be the most correct way of getting the range.

From what i could tell this is currently not easily testable though, right?

Can you elaborate this a bit? What do you want to do exactly?

[jdrst]

Can you elaborate on the use case for this request? Do you want to generate a diff between a change's commit and the previous release?

yes, as stated:

i would like to add links to diffs of certain files to the changelog.

in my case this would be achieveable via the commit hashes

Can you elaborate this a bit? What do you want to do exactly?

my proposal would put the logic in fn process_repository - looking at references i couldn't find a test that runs that logic.
a quick check locally with an actual project produced results that were not what i was expecting and i'd like to look at it more isolated

[orhun]

I think we can use test fixtures for testing this functionality in the first phase. And yes, I think process_repository is a good place to implement it 👍🏼

[jdrst]

Ah, missed those. It looks like commits are created "on the fly" there. Any quick idea how i can deterministically create the same commit sha every time? Apply patches?
Or is there another way to put those in the expected.md?

I'll try to get a pr going some time soon.

[orhun]

Good question. I think applying patches might work. Or we can install a tool like lucky_commit in CI to generate hashes like 1010101 etc. It might be a bit of an overkill though.

Edit: Just do this in commit.sh to use an already existing repo:

#!/usr/bin/env bash
set -e

git remote add origin https://github.com/orhun/git-cliff-readme-example
git pull origin master
git fetch --tags

[jdrst]

still on my list, just didn't get to it yet. apologies for the delay

[orhun]

no worries :)

[jdrst]

Hey, i found the time to work a bit on this. I feel it's still a bit more complicated than thought :D

While playing around with it, i figured that you don't necessarily want the whole commit id but the short id, especially for display purposes. So i added a short_id.
I also added the commit range per release. Otherwise it's wrong when cliff is called for more than one release.

You can see the current state here (no docs yet etc.)

Running that on the https://github.com/orhun/git-cliff-readme-example repo with this cliff.toml:

# git-cliff ~ configuration file
# https://git-cliff.org/docs/configuration

[changelog]
# A Tera template to be rendered for each release in the changelog.
# See https://keats.github.io/tera/docs/#introduction
body = """
{{ commit_range.from_short }}..{{ commit_range.to_short }}
{{ commit_range.from }}..{{ commit_range.to }}
{% if version %}\
    ## [{{ version | trim_start_matches(pat="v") }}] - {{ timestamp | date(format="%Y-%m-%d") }}
{% else %}\
    ## [unreleased]
{% endif %}\
{% for group, commits in commits | group_by(attribute="group") %}
    ### {{ group | upper_first }}
    {% for group, commits in commits | group_by(attribute="scope") %}
        #### {{ group | upper_first }}
        {% for commit in commits %}
            - {{ commit.message | upper_first }}\
        {% endfor %}
    {% endfor %}\
{% endfor %}\n
"""

[git]
# An array of regex based parsers for extracting data from the commit message.
# Assigns commits to groups.
# Optionally sets the commit's scope and can decide to exclude commits from further processing.
commit_parsers = [
    { message = "^feat", group = "Features", default_scope = "app" },
    { message = "^fix", group = "Bug Fixes", scope = "cli" },
]

Generates the following:

# Changelog

All notable changes to this project will be documented in this file.

a9d4050..df6aef4
a9d4050212a18f6b3bd76e2e41fbb9045d268b80..df6aef41292f3ffe5887754232e6ea7831c50ba5
## [unreleased]

### Features

#### Cache

- Use cache while fetching pages

#### Config

- Support multiple file formats

e4fd3cf..06412ac
e4fd3cf8e2e6f49c0b57f66416e886c37cbb3715..06412ac1dd4071006c465dde6597a21d4367a158
## [1.0.1] - 2021-07-18

### Chore

#### Release

- Add release script

### Refactor

#### Parser

- Expose string functions

a78bc36..ad27b43
a78bc368e9ee382a3016c0c4bab41f7de4503bcd..ad27b43e8032671afb4809a1a3ecf12f45c60e0e
## [1.0.0] - 2021-07-18

### Bug Fixes

#### Cli

- Rename help argument due to conflict

### Features

#### Parser

- Add ability to parse arrays

### Docs

#### Example

- Add tested usage example

#### Project

- Add README.md

<!-- generated by git-cliff -->

So far so good. This seems to work.

Problematic is, that there's also other ways to create a commit. Either from a String or from id and message String.

The latter seems to only be used in test cases, which should be unproblematic. The former get's used in case of custom commits though.
If parsed from a string that includes a hash it's complicated to deduce the short id. I haven't dug into it but it's probably somewhere inbetween parsing the other commits ourselves to figure out the shortest unique id (which is configurable via core.abbrev - see git_object_short_id) or utilizing git (temporarily adding that commit?) to get the short_id. Of course we could also just ignore custom commits. That imho kinda renders the feature useless in that case though :/

What do you think is the best option here?

I also could not figure out how to run this integration_test. Is that in use somewhere?

/edit: correct template output

[orhun]

Hey again, thanks for the PoC! Seems fine to me, please send a PR!

That imho kinda renders the feature useless in that case though :/

I guess that's fine - it's a tradeoff when custom commits are used.

I also could not figure out how to run this integration_test. Is that in use somewhere?

Can you elaborate this? Are you trying to update that test to include coverage for this feature.I think it should be run when you do cargo test.

[jdrst]

Can you elaborate this? Are you trying to update that test to include coverage for this feature.I think it should be run when you do cargo test.

nvm this. I got something mixed up.