diff --git a/git-cliff-core/src/changelog.rs b/git-cliff-core/src/changelog.rs index 016243a23f..43870e6067 100644 --- a/git-cliff-core/src/changelog.rs +++ b/git-cliff-core/src/changelog.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::io::{Read, Write}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -52,6 +52,19 @@ impl<'a> Changelog<'a> { /// Builds a changelog from releases and config. fn build(releases: Vec>, config: Config) -> Result { let trim = config.changelog.trim; + let mut additional_context: HashMap = HashMap::new(); + let mut seen_groups = HashSet::new(); + let parser_groups: Vec = config + .git + .commit_parsers + .iter() + .filter_map(|p| p.group.clone()) + .filter(|g| seen_groups.insert(g.clone())) + .collect(); + additional_context.insert( + String::from("commit_parsers_groups"), + serde_json::to_value(parser_groups)?, + ); Ok(Self { releases, header_template: match &config.changelog.header { @@ -64,7 +77,7 @@ impl<'a> Changelog<'a> { None => None, }, config, - additional_context: HashMap::new(), + additional_context, }) } @@ -1552,6 +1565,86 @@ chore(deps): fix broken deps Ok(()) } + /// Regression for : commit + /// groups should be rendered in the order in which they first appear in + /// `commit_parsers`, not alphabetically. + #[test] + fn changelog_group_order_matches_commit_parsers() -> Result<()> { + // This is the reproducer from the issue: a small `commit_parsers` + // list with named groups that would be alphabetized by the built-in + // `group_by` filter. + let config = Config { + changelog: ChangelogConfig { + header: None, + body: String::from( + "{% for entry in commits | commit_groups(groups=commit_parsers_groups) %}### \ + {{ entry.name }}\n{% for commit in entry.commits %}- {{ commit.message \ + }}\n{% endfor %}{% endfor %}", + ), + footer: None, + trim: true, + postprocessors: Vec::new(), + render_always: false, + output: None, + }, + git: GitConfig { + conventional_commits: true, + filter_unconventional: false, + commit_parsers: vec![ + CommitParser { + message: Regex::new("^feat").ok(), + group: Some(String::from(":rocket: New features")), + ..Default::default() + }, + CommitParser { + message: Regex::new("^fix").ok(), + group: Some(String::from(":bug: Bug fixes")), + ..Default::default() + }, + CommitParser { + message: Regex::new("^perf").ok(), + group: Some(String::from(":zap: Performance")), + ..Default::default() + }, + CommitParser { + message: Regex::new("^chore").ok(), + group: Some(String::from(":gear: Miscellaneous")), + ..Default::default() + }, + ], + ..Default::default() + }, + remote: RemoteConfig::default(), + bump: Bump::default(), + }; + + // Commits arrive in an order whose group names sort alphabetically + // (`:bug:` < `:gear:` < `:rocket:` < `:zap:`), which is the wrong + // order. The filter is expected to use the `commit_parsers` order + // instead. + let release = Release { + version: None, + commits: vec![ + Commit::new(String::from("1"), String::from("chore: misc")), + Commit::new(String::from("2"), String::from("fix: a bug")), + Commit::new(String::from("3"), String::from("perf: faster")), + Commit::new(String::from("4"), String::from("feat: shiny new thing")), + ], + ..Default::default() + }; + + let changelog = Changelog::new(vec![release], config, None)?; + let mut out = Vec::new(); + changelog.generate(&mut out)?; + let rendered = String::from_utf8(out).unwrap_or_default(); + + let expected = "### :rocket: New features\n- shiny new thing\n### :bug: Bug fixes\n- a \ + bug\n### :zap: Performance\n- faster\n### :gear: Miscellaneous\n- misc\n"; + assert_eq!(expected, rendered); + + Ok(()) + } + #[test] fn changelog_adds_additional_context() -> Result<()> { let (mut config, releases) = get_test_data(); diff --git a/git-cliff-core/src/template.rs b/git-cliff-core/src/template.rs index f8fdd4e67a..74b6244016 100644 --- a/git-cliff-core/src/template.rs +++ b/git-cliff-core/src/template.rs @@ -1,8 +1,10 @@ use std::collections::{HashMap, HashSet}; use std::error::Error as ErrorImpl; +use indexmap::IndexMap; use regex::Regex; use serde::Serialize; +use serde_json::json; use tera::{Context as TeraContext, Result as TeraResult, Tera, Value, ast}; use crate::config::TextProcessor; @@ -43,6 +45,7 @@ impl Template { tera.register_filter("split_regex", Self::split_regex); tera.register_filter("replace_regex", Self::replace_regex); tera.register_filter("find_regex", Self::find_regex); + tera.register_filter("commit_groups", Self::commit_groups); Ok(Self { name: name.to_string(), @@ -51,6 +54,67 @@ impl Template { }) } + /// Groups commits by their `group` field while preserving ordering. + /// + /// Behaves like Tera's built-in `group_by(attribute="group")` filter, but + /// yields entries as an array so iteration order is well-defined. Each + /// entry has a `name` (the group name) and `commits` (the matching + /// commits, in their original order). + /// + /// When the optional `groups` argument is provided (an array of group + /// names, typically the order of `commit_parsers` in the configuration), + /// the output is sorted to match that order. Any group not listed in + /// `groups` is appended after the listed ones, in first-appearance order. + /// When `groups` is omitted, the output preserves the first-appearance + /// order of groups in the input list (which mirrors commit chronology). + /// + /// Commits whose `group` is null or missing are skipped, matching the + /// behavior of the built-in `group_by` filter. + fn commit_groups(value: &Value, args: &HashMap) -> TeraResult { + let arr = tera::try_get_value!("commit_groups", "value", Vec, value); + + let group_priority: Option> = match args.get("groups") { + Some(val) => { + let groups = + tera::try_get_value!("commit_groups", "groups", Vec, val.clone()); + let mut map = HashMap::with_capacity(groups.len()); + for (idx, name) in groups.into_iter().enumerate() { + map.entry(name).or_insert(idx); + } + Some(map) + } + None => None, + }; + + let mut grouped: IndexMap> = IndexMap::new(); + for val in arr { + let key_val = match val.get("group") { + Some(v) if !v.is_null() => v.clone(), + _ => continue, + }; + let str_key = match key_val.as_str() { + Some(k) => k.to_owned(), + None => format!("{key_val}"), + }; + grouped.entry(str_key).or_default().push(val); + } + + if let Some(priority) = &group_priority { + let next_priority = priority.len(); + grouped.sort_by(|a_name, _, b_name, _| { + let a = priority.get(a_name).copied().unwrap_or(next_priority); + let b = priority.get(b_name).copied().unwrap_or(next_priority); + a.cmp(&b) + }); + } + + let result: Vec = grouped + .into_iter() + .map(|(name, commits)| json!({ "name": name, "commits": commits })) + .collect(); + Ok(tera::to_value(result)?) + } + /// Filter for making the first character of a string uppercase. fn upper_first_filter(value: &Value, _: &HashMap) -> TeraResult { let mut s = tera::try_get_value!("upper_first_filter", "value", String, value); @@ -404,4 +468,105 @@ mod test { assert_eq!("[hello, world,, hello, universe]", r); Ok(()) } + + /// Builds a release whose commits would be sorted alphabetically by the + /// built-in `group_by` filter. Reproduces the scenario from + /// . + fn release_with_emoji_groups() -> Release<'static> { + let mut release = get_fake_release_data(); + release.commits = vec![ + { + let mut c = Commit::new(String::from("000001"), String::from("perf: speed")); + c.group = Some(String::from("\u{26A1} Performance")); + c + }, + { + let mut c = Commit::new(String::from("000002"), String::from("fix: bug")); + c.group = Some(String::from("\u{1F41B} Bug Fixes")); + c + }, + { + let mut c = Commit::new(String::from("000003"), String::from("feat: new")); + c.group = Some(String::from("\u{1F680} Features")); + c + }, + { + let mut c = Commit::new(String::from("000004"), String::from("feat: another")); + c.group = Some(String::from("\u{1F680} Features")); + c + }, + ]; + release + } + + #[test] + fn test_commit_groups_filter_preserves_first_appearance_when_no_groups() -> Result<()> { + let template = "{% for entry in commits | commit_groups %}{{ entry.name }}|{{ \ + entry.commits | length }};{% endfor %}"; + let template = Template::new("test", template.to_string(), true)?; + let release = release_with_emoji_groups(); + let r = template.render(&release, Option::>::None.as_ref(), &[ + ])?; + assert_eq!( + "\u{26A1} Performance|1;\u{1F41B} Bug Fixes|1;\u{1F680} Features|2;", + r + ); + Ok(()) + } + + #[test] + fn test_commit_groups_filter_uses_groups_argument() -> Result<()> { + let template = "{% for entry in commits | commit_groups(groups=order) %}{{ entry.name \ + }}|{{ entry.commits | length }};{% endfor %}"; + let template = Template::new("test", template.to_string(), true)?; + let release = release_with_emoji_groups(); + let mut additional: HashMap<&str, Vec<&str>> = HashMap::new(); + additional.insert("order", vec![ + "\u{1F680} Features", + "\u{1F41B} Bug Fixes", + "\u{26A1} Performance", + ]); + let r = template.render(&release, Some(&additional), &[])?; + assert_eq!( + "\u{1F680} Features|2;\u{1F41B} Bug Fixes|1;\u{26A1} Performance|1;", + r + ); + Ok(()) + } + + #[test] + fn test_commit_groups_filter_appends_unknown_groups() -> Result<()> { + let template = + "{% for entry in commits | commit_groups(groups=order) %}{{ entry.name }};{% endfor %}"; + let template = Template::new("test", template.to_string(), true)?; + let release = release_with_emoji_groups(); + let mut additional: HashMap<&str, Vec<&str>> = HashMap::new(); + additional.insert("order", vec!["\u{1F680} Features"]); + let r = template.render(&release, Some(&additional), &[])?; + assert_eq!( + "\u{1F680} Features;\u{26A1} Performance;\u{1F41B} Bug Fixes;", + r + ); + Ok(()) + } + + #[test] + fn test_commit_groups_filter_skips_null_groups() -> Result<()> { + let template = "{% for entry in commits | commit_groups %}{{ entry.name }}|{{ \ + entry.commits | length }};{% endfor %}"; + let template = Template::new("test", template.to_string(), true)?; + let mut release = get_fake_release_data(); + release.commits = vec![ + { + let mut c = Commit::new(String::from("a"), String::from("a")); + c.group = Some(String::from("kept")); + c + }, + Commit::new(String::from("b"), String::from("b")), + ]; + let r = template.render(&release, Option::>::None.as_ref(), &[ + ])?; + assert_eq!("kept|1;", r); + Ok(()) + } }