Commit footers have values but no names

[hawkw]

Describe the bug

When a conventional commit has footers (such as Signed-off-by: Someone <email>), the footers array provided to the changelog template contains the value (the part after the :) but not the name of the footer. This makes it difficult to implement any kind of special templating based on the name of the footer. For example, if I want to add Signed-off-by emails to a changelog entry, but not Reviewed-by, it's currently impossible to determine what commit footer that value came from.

To Reproduce

In a repository where commits contain multiple footers with different names, create a template with {{ commit.footers }} in it somewhere. Note that only the values of the footer are output.

Expected behavior

The entire footer (including the name) should be exposed to the template. This could either be by making each entry in the commit.footers array be the entire footer value, including both the name and the value after the colon, or by changing commit.footers from an array to a map of footer names to footer values.

System

• OS Information:

  :; uname -a
  Linux noctis 5.17.3 #1-NixOS SMP PREEMPT Wed Apr 13 17:27:43 UTC 2022 x86_64 GNU/Linux
  

• Project Version:

  :; git-cliff --version
  git-cliff 0.7.0
  

Additional context

While looking at the code for serializing commits as JSON, I noticed that the Serialize impl maps over the list of footers and calls f.value():

git-cliff/git-cliff-core/src/commit.rs

Lines 221 to 226 in 7d0786c

	&conv
	.footers()
	.to_vec()
	.iter()
	.map(|f| f.value())
	.collect::<Vec<&str>>(),

Although I'm not familiar with the git_conventional crate's API, my assumption is that this is discarding the footer's name. :)

I'm happy to open a PR changing this behavior, which I imagine would be fairly straightforward. The one design decision is what we should output instead: is it preferable to emit the footers as a map of footer name to footer value (which could break existing templates, but seems nicer), or as an array of the footer's name and value as a single string? Or, is there some other approach that I haven't thought of?

[orhun]

Hello, thanks for submitting this issue and the detailed explanation! 🐻

I would like to go over the possible options that you suggested and talk about the pros and cons.

1. Single string solution

According to git_conventional API, footers consist of 3 fields:

pub struct Footer<'a> {
    token: FooterToken<'a>,
    sep: FooterSeparator,
    value: &'a str,
}

(Note that all the fields are private.)

So doing this would be enough:

commit.serialize_field(
    "footers",
    &conv
        .footers()
        .to_vec()
        .iter()
        .map(|f| {
            format!("{}{} {}", f.token(), f.separator(), f.value())
        })
        .collect::<Vec<String>>(),
)?;

Although this looks like a feasible solution and the context is compatible with the previous versions of git-cliff, I think that we lose a bit of flexibility here. You made a good point about using different part of footer values. It would be nice if we could use footers as-is and access the fields by e.g. footer.token, footer.value, etc. However, as I pointed out, git_conventional::Footer fields are private. So we need to define our own Footer type and serialize it.

2. Footer object method

Simply define the following struct:

/// Commit footer.
#[derive(Debug, Clone, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
pub struct Footer {
	/// Token of the footer.
	pub token:     String,
	/// The separator between the footer token and its value.
	pub separator: String,
	/// The value of the footer.
	pub value:     String,
	/// A flag to signal that the footer describes a breaking change.
	pub breaking:  bool,
}

impl From<git_conventional::Footer<'_>> for Footer {
	fn from(f: git_conventional::Footer) -> Self {
		Self {
			token:     f.token().to_string(),
			separator: f.separator().to_string(),
			value:     f.value().to_string(),
			breaking:  f.breaking(),
		}
	}
}

Then serialize this type instead:

commit.serialize_field(
    "footers",
    &conv
        .footers()
        .to_vec()
        .into_iter()
        .map(Footer::from)
        .collect::<Vec<Footer>>(),
)?;

It can easily be used in templates like this:

{% for group, commits in commits | group_by(attribute="group") %}
    ### {{ group | upper_first }}
    {% for commit in commits %}
        - {% if commit.breaking %}[**breaking**] {% endif %}{{ commit.message | upper_first }}
         {% for footer in commit.footers %}\
            \t- {{ footer.token }}{{ footer.separator }} {{ footer.value }}
        {% endfor %}\
    {% endfor %}
{% endfor %}\n

The drawback is it is not backwards compatible and if you try to use commit.footers directly you will get [object] instead of a string. But it is a good compromise for increasing the templating flexibility I would say.

---

I'm happy to open a PR changing this behavior, which I imagine would be fairly straightforward. The one design decision is what we should output instead: is it preferable to emit the footers as a map of footer name to footer value (which could break existing templates, but seems nicer), or as an array of the footer's name and value as a single string? Or, is there some other approach that I haven't thought of?

I think the Footer object approach is the way to go. Feel free to share your thoughts and move on with the PR! 🐻

[hawkw]

I think the Footer object approach is the way to go. Feel free to share your thoughts and move on with the PR! 🐻

That's what I thought, too, thanks for the confirmation. The breaking change is kind of unfortunate, though, so I wanted to ask.

I suppose a third option is to keep the existing commit.footers, and add a new commit.named_footers (or something) that includes the keys and values. IMO, this is a bit more awkward, but it wouldn't break existing templates.

[orhun]

I suppose a third option is to keep the existing commit.footers, and add a new commit.named_footers (or something) that includes the keys and values. IMO, this is a bit more awkward, but it wouldn't break existing templates.

I would say that commit.footers would be more appropriate and also it feels like it is how it should have been implemented in the beginning. Surprisingly, no one caught this until now!