From d98195bc4d3e18da05d1fb29c56528f6180a9b2f Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sat, 15 Jun 2024 22:27:21 +0200 Subject: [PATCH 1/5] refactor: clean up some code --- git-cliff-core/src/changelog.rs | 11 +-- git-cliff-core/src/repo.rs | 58 ++++++------ git-cliff/src/lib.rs | 160 ++++++++++++++------------------ 3 files changed, 106 insertions(+), 123 deletions(-) diff --git a/git-cliff-core/src/changelog.rs b/git-cliff-core/src/changelog.rs index 7707f37..712d4ef 100644 --- a/git-cliff-core/src/changelog.rs +++ b/git-cliff-core/src/changelog.rs @@ -494,7 +494,7 @@ impl<'a> Changelog<'a> { } /// Generates the changelog and writes it to the given output. - pub fn generate(&self, out: &mut W) -> Result<()> { + pub fn generate(&self, out: &mut W) -> Result<()> { debug!("Generating changelog..."); let postprocessors = self .config @@ -510,8 +510,7 @@ impl<'a> Changelog<'a> { } } } - let mut releases = self.releases.clone(); - for release in releases.iter_mut() { + for release in &self.releases { let write_result = write!( out, "{}", @@ -533,7 +532,7 @@ impl<'a> Changelog<'a> { "{}", footer_template.render( &Releases { - releases: &releases, + releases: &self.releases, }, Some(&self.additional_context), &postprocessors, @@ -549,7 +548,7 @@ impl<'a> Changelog<'a> { } /// Generates a changelog and prepends it to the given changelog. - pub fn prepend( + pub fn prepend( &self, mut changelog: String, out: &mut W, @@ -564,7 +563,7 @@ impl<'a> Changelog<'a> { } /// Prints the changelog context to the given output. - pub fn write_context(&self, out: &mut W) -> Result<()> { + pub fn write_context(&self, out: &mut W) -> Result<()> { let output = Releases { releases: &self.releases, } diff --git a/git-cliff-core/src/repo.rs b/git-cliff-core/src/repo.rs index 8723a64..771c5c4 100644 --- a/git-cliff-core/src/repo.rs +++ b/git-cliff-core/src/repo.rs @@ -45,14 +45,14 @@ impl Repository { /// Sorts the commits by their time. pub fn commits( &self, - range: Option, - include_path: Option>, - exclude_path: Option>, + range: Option<&str>, + include_path: Option<&[Pattern]>, + exclude_path: Option<&[Pattern]>, ) -> Result> { let mut revwalk = self.inner.revwalk()?; revwalk.set_sorting(Sort::TOPOLOGICAL)?; if let Some(range) = range { - revwalk.push_range(&range)?; + revwalk.push_range(range)?; } else { revwalk.push_head()?; } @@ -62,31 +62,31 @@ impl Repository { .collect(); if include_path.is_some() || exclude_path.is_some() { commits.retain(|commit| { - if let Ok(prev_commit) = commit.parent(0) { - if let Ok(diff) = self.inner.diff_tree_to_tree( - commit.tree().ok().as_ref(), - prev_commit.tree().ok().as_ref(), - None, - ) { - return diff - .deltas() - .filter_map(|delta| delta.new_file().path()) - .any(|new_file_path| { - if let Some(include_path) = &include_path { - include_path - .iter() - .any(|glob| glob.matches_path(new_file_path)) - } else if let Some(exclude_path) = &exclude_path { - !exclude_path - .iter() - .any(|glob| glob.matches_path(new_file_path)) - } else { - false - } - }); - } - } - false + let Ok(prev_commit) = commit.parent(0) else { + return false; + }; + let Ok(diff) = self.inner.diff_tree_to_tree( + commit.tree().ok().as_ref(), + prev_commit.tree().ok().as_ref(), + None, + ) else { + return false; + }; + diff.deltas() + .filter_map(|delta| delta.new_file().path()) + .any(|new_file_path| { + if let Some(include_path) = include_path { + return include_path + .iter() + .any(|glob| glob.matches_path(new_file_path)); + } + if let Some(exclude_path) = exclude_path { + return !exclude_path + .iter() + .any(|glob| glob.matches_path(new_file_path)); + } + unreachable!() + }) }); } Ok(commits) diff --git a/git-cliff/src/lib.rs b/git-cliff/src/lib.rs index 61a835f..a042174 100644 --- a/git-cliff/src/lib.rs +++ b/git-cliff/src/lib.rs @@ -44,11 +44,8 @@ use std::fs::{ self, File, }; -use std::io::{ - self, - Write, -}; -use std::path::PathBuf; +use std::io; +use std::path::Path; use std::time::{ SystemTime, UNIX_EPOCH, @@ -88,29 +85,26 @@ fn process_repository<'a>( let mut tags = repository.tags(&config.git.tag_pattern, args.topo_order)?; let skip_regex = config.git.skip_tags.as_ref(); let ignore_regex = config.git.ignore_tags.as_ref(); - tags = tags - .into_iter() - .filter(|(_, name)| { - // Keep skip tags to drop commits in the later stage. - let skip = skip_regex.map(|r| r.is_match(name)).unwrap_or_default(); + tags.retain(|_, name| { + // Keep skip tags to drop commits in the later stage. + let skip = skip_regex.is_some_and(|r| r.is_match(name)); + if skip { + return true; + } - let ignore = ignore_regex - .map(|r| { - if r.as_str().trim().is_empty() { - return false; - } + let ignore = ignore_regex.is_some_and(|r| { + if r.as_str().trim().is_empty() { + return false; + } - let ignore_tag = r.is_match(name); - if ignore_tag { - trace!("Ignoring release: {}", name) - } - ignore_tag - }) - .unwrap_or_default(); - - skip || !ignore - }) - .collect(); + let ignore_tag = r.is_match(name); + if ignore_tag { + trace!("Ignoring release: {}", name) + } + ignore_tag + }); + !ignore + }); if !config.remote.github.is_set() { match repository.upstream_remote() { @@ -211,14 +205,12 @@ fn process_repository<'a>( } } let mut commits = repository.commits( - commit_range, - args.include_path.clone(), - args.exclude_path.clone(), + commit_range.as_deref(), + args.include_path.as_deref(), + args.exclude_path.as_deref(), )?; if let Some(commit_limit_value) = config.git.limit_commits { - commits = commits - .drain(..commits.len().min(commit_limit_value)) - .collect(); + commits.truncate(commit_limit_value); } // Update tags. @@ -237,21 +229,17 @@ fn process_repository<'a>( // Process releases. let mut releases = vec![Release::default()]; - let mut release_index = 0; let mut previous_release = Release::default(); let mut first_processed_tag = None; for git_commit in commits.iter().rev() { + let release = releases.last_mut().unwrap(); let commit = Commit::from(git_commit); let commit_id = commit.id.to_string(); - if args.sort == Sort::Newest { - releases[release_index].commits.insert(0, commit); - } else { - releases[release_index].commits.push(commit); - } + release.commits.push(commit); if let Some(tag) = tags.get(&commit_id) { - releases[release_index].version = Some(tag.to_string()); - releases[release_index].commit_id = Some(commit_id); - releases[release_index].timestamp = if args.tag.as_deref() == Some(tag) { + release.version = Some(tag.to_string()); + release.commit_id = Some(commit_id); + release.timestamp = if args.tag.as_deref() == Some(tag) { SystemTime::now() .duration_since(UNIX_EPOCH)? .as_secs() @@ -263,36 +251,40 @@ fn process_repository<'a>( first_processed_tag = Some(tag); } previous_release.previous = None; - releases[release_index].previous = Some(Box::new(previous_release)); - previous_release = releases[release_index].clone(); + release.previous = Some(Box::new(previous_release)); + previous_release = release.clone(); releases.push(Release::default()); - release_index += 1; } } - if release_index > 0 { + debug_assert!(!releases.is_empty()); + + if releases.len() > 1 { previous_release.previous = None; - releases[release_index].previous = Some(Box::new(previous_release)); + releases.last_mut().unwrap().previous = Some(Box::new(previous_release)); + } + + if args.sort == Sort::Newest { + for release in &mut releases { + release.commits.reverse(); + } } // Add custom commit messages to the latest release. if let Some(custom_commits) = &args.with_commit { - if let Some(latest_release) = releases.iter_mut().last() { - custom_commits.iter().for_each(|message| { - latest_release - .commits - .push(Commit::from(message.to_string())) - }); - } + releases + .last_mut() + .unwrap() + .commits + .extend(custom_commits.iter().cloned().map(Commit::from)); } // Set the previous release if the first release does not have one set. - if !releases.is_empty() && - releases - .first() - .and_then(|r| r.previous.as_ref()) - .and_then(|p| p.version.as_ref()) - .is_none() + if releases[0] + .previous + .as_ref() + .and_then(|p| p.version.as_ref()) + .is_none() { // Get the previous tag of the first processed tag in the release loop. let first_tag = first_processed_tag @@ -532,6 +524,16 @@ pub fn run(mut args: Opt) -> Result<()> { let mut changelog = Changelog::new(releases, &config)?; // Print the result. + let mut out: Box = if let Some(path) = &args.output { + if path == Path::new("-") { + Box::new(io::stdout()) + } else { + Box::new(io::BufWriter::new(File::create(path)?)) + } + } else { + Box::new(io::stdout()) + }; + let out = &mut *out; if args.bump || args.bumped_version { let next_version = if let Some(next_version) = changelog.bump_version()? { next_version @@ -544,40 +546,22 @@ pub fn run(mut args: Opt) -> Result<()> { return Ok(()); }; if args.bumped_version { - if let Some(path) = args.output { - let mut output = File::create(path)?; - output.write_all(next_version.as_bytes())?; - } else { - println!("{next_version}"); - } + writeln!(out, "{next_version}")?; return Ok(()); } } + if args.context { - return if let Some(path) = args.output { - let mut output = File::create(path)?; - changelog.write_context(&mut output) - } else { - changelog.write_context(&mut io::stdout()) - }; + changelog.write_context(out)?; + return Ok(()); } - if let Some(ref path) = args.prepend { - changelog.prepend(fs::read_to_string(path)?, &mut File::create(path)?)?; + + if let Some(path) = &args.prepend { + changelog.prepend(fs::read_to_string(path)?, out)?; } - if let Some(path) = args.output { - let mut output: Box = if path == PathBuf::from("-") { - Box::new(io::stdout()) - } else { - Box::new(File::create(path)?) - }; - if args.context { - changelog.write_context(&mut output) - } else { - changelog.generate(&mut output) - } - } else if args.prepend.is_none() { - changelog.generate(&mut io::stdout()) - } else { - Ok(()) + if args.output.is_some() || args.prepend.is_none() { + changelog.generate(out)?; } + + Ok(()) } From f4afa8cdafbb37c43bf4d617c81f4dc0e5dd8a29 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Mon, 24 Jun 2024 20:27:22 +0200 Subject: [PATCH 2/5] Update git-cliff/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Orhun Parmaksız --- git-cliff/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-cliff/src/lib.rs b/git-cliff/src/lib.rs index a042174..9e9a96a 100644 --- a/git-cliff/src/lib.rs +++ b/git-cliff/src/lib.rs @@ -557,7 +557,7 @@ pub fn run(mut args: Opt) -> Result<()> { } if let Some(path) = &args.prepend { - changelog.prepend(fs::read_to_string(path)?, out)?; + changelog.prepend(fs::read_to_string(path)?, &mut File::create(path)?)?; } if args.output.is_some() || args.prepend.is_none() { changelog.generate(out)?; From 8e084ad59de80370fe38e671554be1fc3833eb4f Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Mon, 24 Jun 2024 20:32:04 +0200 Subject: [PATCH 3/5] review --- git-cliff/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/git-cliff/src/lib.rs b/git-cliff/src/lib.rs index 9e9a96a..cc124a0 100644 --- a/git-cliff/src/lib.rs +++ b/git-cliff/src/lib.rs @@ -533,7 +533,6 @@ pub fn run(mut args: Opt) -> Result<()> { } else { Box::new(io::stdout()) }; - let out = &mut *out; if args.bump || args.bumped_version { let next_version = if let Some(next_version) = changelog.bump_version()? { next_version @@ -552,15 +551,16 @@ pub fn run(mut args: Opt) -> Result<()> { } if args.context { - changelog.write_context(out)?; + changelog.write_context(&mut out)?; return Ok(()); } if let Some(path) = &args.prepend { - changelog.prepend(fs::read_to_string(path)?, &mut File::create(path)?)?; + let mut out = io::BufWriter::new(File::create(path)?); + changelog.prepend(fs::read_to_string(path)?, &mut out)?; } if args.output.is_some() || args.prepend.is_none() { - changelog.generate(out)?; + changelog.generate(&mut out)?; } Ok(()) From 68b7e79e75af09df45f31272a1a893d5da787ff0 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Mon, 24 Jun 2024 20:37:10 +0200 Subject: [PATCH 4/5] fmt --- git-cliff-core/src/changelog.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/git-cliff-core/src/changelog.rs b/git-cliff-core/src/changelog.rs index dc1c3a9..61f1173 100644 --- a/git-cliff-core/src/changelog.rs +++ b/git-cliff-core/src/changelog.rs @@ -425,14 +425,16 @@ impl<'a> Changelog<'a> { serde_json::to_value(self.config.remote.clone())?, ); #[cfg(feature = "github")] - let (github_commits, github_pull_requests) = if self.config.remote.github.is_set() { + let (github_commits, github_pull_requests) = if self.config.remote.github.is_set() + { self.get_github_metadata() .expect("Could not get github metadata") } else { (vec![], vec![]) }; #[cfg(feature = "gitlab")] - let (gitlab_commits, gitlab_merge_request) = if self.config.remote.gitlab.is_set() { + let (gitlab_commits, gitlab_merge_request) = if self.config.remote.gitlab.is_set() + { self.get_gitlab_metadata() .expect("Could not get gitlab metadata") } else { From b6d2a219c64cad45f3d659826c961fa16b957b0e Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Wed, 26 Jun 2024 00:59:23 +0200 Subject: [PATCH 5/5] fix: read before opening the file to prepend --- git-cliff/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git-cliff/src/lib.rs b/git-cliff/src/lib.rs index 057cd08..171af59 100644 --- a/git-cliff/src/lib.rs +++ b/git-cliff/src/lib.rs @@ -569,8 +569,9 @@ pub fn run(mut args: Opt) -> Result<()> { } if let Some(path) = &args.prepend { + let changelog_before = fs::read_to_string(path)?; let mut out = io::BufWriter::new(File::create(path)?); - changelog.prepend(fs::read_to_string(path)?, &mut out)?; + changelog.prepend(changelog_before, &mut out)?; } if args.output.is_some() || args.prepend.is_none() { changelog.generate(&mut out)?;