Reorganize changeset handling

crates/knope-versioning/src/action.rs

@@ -7,6 +7,9 @@ pub enum Action {
         path: RelativePathBuf,
         content: String,
     },
+    RemoveFile {
+        path: RelativePathBuf,
+    },
     AddTag {
         tag: String,
     },

crates/knope/src/fs.rs

@@ -54,6 +54,18 @@ pub(crate) fn read_to_string<P: AsRef<Path> + Into<PathBuf>>(path: P) -> Result<
     })
 }
 
+pub(crate) fn remove_file(dry_run: DryRun, path: &Path) -> Result<(), Error> {
+    if let Some(stdout) = dry_run {
+        writeln!(stdout, "Would delete {}", path.display()).map_err(Error::Stdout)
+    } else {
+        trace!("Removing file {}", path.display());
+        std::fs::remove_file(path).map_err(|source| Error::Remove {
+            path: path.into(),
+            source,
+        })
+    }
+}
+
 #[derive(Debug, Diagnostic, Error)]
 pub(crate) enum Error {
     #[error("Error writing to {path}: {source}")]
@@ -76,6 +88,16 @@ pub(crate) enum Error {
         #[source]
         source: io::Error,
     },
+    #[error("Error removing {path}: {source}")]
+    #[diagnostic(
+        code(fs::remove),
+        help("Make sure you have permission to write to this file.")
+    )]
+    Remove {
+        path: PathBuf,
+        #[source]
+        source: io::Error,
+    },
     #[error("Error writing to stdout: {0}")]
     Stdout(#[source] io::Error),
 }

crates/knope/src/step/releases/changesets.rs

@@ -1,13 +1,13 @@
-use std::{collections::HashSet, io::Write, path::PathBuf};
+use std::{io::Write, path::PathBuf};
 
-use changesets::{ChangeSet, UniqueId, Versioning};
+use changesets::{UniqueId, Versioning};
 use inquire::{MultiSelect, Select};
 use itertools::Itertools;
-use knope_versioning::changes::{ChangeType, CHANGESET_DIR, DEFAULT_PACKAGE_NAME};
+use knope_versioning::changes::{Change, ChangeType, CHANGESET_DIR, DEFAULT_PACKAGE_NAME};
 use miette::Diagnostic;
 
 use super::Package;
-use crate::{dry_run::DryRun, fs, prompt, state::RunType};
+use crate::{fs, prompt, state::RunType};
 
 pub(crate) fn create_change_file(run_type: RunType) -> Result<RunType, Error> {
     let state = match run_type {
@@ -80,58 +80,27 @@ pub(crate) fn create_change_file(run_type: RunType) -> Result<RunType, Error> {
 }
 
 // TODO: Move some of this to knope_versioning
-pub(crate) fn add_releases_from_changeset(
-    packages: Vec<Package>,
-    is_prerelease: bool,
-    dry_run: DryRun,
-) -> Result<Vec<Package>, Error> {
-    let changeset_path = PathBuf::from(CHANGESET_DIR);
-    if !changeset_path.exists() {
-        return Ok(packages);
-    }
-    let releases: Vec<changesets::Release> = ChangeSet::from_directory(&changeset_path)?.into();
-    let mut changesets_deleted = HashSet::new();
-    Ok(packages
+pub(crate) fn changes_from_changesets<'a>(
+    package: &'a Package,
+    releases: &'a [changesets::Release],
+) -> impl Iterator<Item = Change> + 'a {
+    releases
+        .iter()
+        .find(|release| {
+            release.package_name == package.name.as_deref().unwrap_or(DEFAULT_PACKAGE_NAME)
+        })
         .into_iter()
-        .map(|mut package| {
-            if let Some(release_changes) = releases.iter().find(|release| {
-                release.package_name == package.name.as_deref().unwrap_or(DEFAULT_PACKAGE_NAME)
-            }) {
-                package
-                    .pending_changes
-                    .extend(release_changes.changes.clone().into_iter().map(|change| {
-                        let file_name = change.unique_id.to_file_name();
-                        if !changesets_deleted.contains(&file_name) && !is_prerelease {
-                            if let Some(dry_run) = dry_run {
-                                writeln!(
-                                    dry_run,
-                                    "Would delete: {}",
-                                    changeset_path.join(&file_name).display()
-                                )
-                                .ok(); // Truly not the end of the world if stdio fails, and error handling is hard
-                            } else {
-                                std::fs::remove_file(changeset_path.join(&file_name)).ok();
-                            }
-                            changesets_deleted.insert(file_name);
-                        }
-                        change.into()
-                    }));
-            }
-            package
+        .flat_map(|release_changes| {
+            release_changes
+                .changes
+                .clone()
+                .into_iter()
+                .map(Change::from)
         })
-        .collect())
 }
 
 #[derive(Debug, Diagnostic, thiserror::Error)]
 pub(crate) enum Error {
-    #[error(transparent)]
-    #[diagnostic(
-        code(changesets::could_not_read_changeset),
-        help(
-            "This could be a file-system issue or a problem with the formatting of a change file."
-        )
-    )]
-    CouldNotReadChangeSet(#[from] changesets::LoadingError),
     #[error(transparent)]
     #[diagnostic(transparent)]
     Fs(#[from] fs::Error),

crates/knope/src/step/releases/mod.rs

@@ -1,9 +1,14 @@
-use std::collections::BTreeMap;
+use std::{collections::BTreeMap, path::PathBuf};
 
+use ::changesets::ChangeSet;
 use itertools::Itertools;
-use knope_versioning::{Action, PreVersion, StableVersion, Version};
+use knope_versioning::{
+    changes::{ChangeSource, CHANGESET_DIR},
+    Action, PreVersion, StableVersion, Version,
+};
 use miette::Diagnostic;
 pub(crate) use non_empty_map::PrereleaseMap;
+use relative_path::RelativePathBuf;
 
 pub(crate) use self::{
     changelog::Release,
@@ -12,6 +17,7 @@ pub(crate) use self::{
     semver::{bump_version_and_update_state, Rule},
 };
 use crate::{
+    dry_run::DryRun,
     integrations::{
         git,
         git::{create_tag, get_current_versions_from_tags},
@@ -41,55 +47,32 @@ pub(crate) fn prepare_release(
     if state.packages.is_empty() {
         return Err(package::Error::NoDefinedPackages.into());
     }
-    let PrepareRelease {
-        prerelease_label,
-        allow_empty,
-        ignore_conventional_commits,
-    } = prepare_release;
-    if !ignore_conventional_commits {
-        state.packages = state
-            .packages
-            .into_iter()
-            .map(|mut package| {
-                conventional_commits::get_conventional_commits_after_last_stable_version(
-                    &package.name,
-                    package.versioning.scopes.as_ref(),
-                    &package.versioning.changelog_sections,
-                    state.verbose,
-                    &state.all_git_tags,
-                )
-                .map(|pending_changes| {
-                    package.pending_changes = pending_changes;
-                    package
-                })
-            })
-            .try_collect()?;
-    }
-    state.packages = changesets::add_releases_from_changeset(
-        state.packages,
-        prerelease_label.is_some(),
-        &mut dry_run_stdout,
-    )
-    .map_err(Error::from)
-    .and_then(|packages| {
-        packages
-            .into_iter()
-            .map(|package| {
-                package
-                    .write_release(
-                        prerelease_label,
-                        &state.all_git_tags,
-                        &mut dry_run_stdout,
-                        state.verbose,
-                    )
-                    .map_err(Error::from)
-            })
-            .collect()
-    })?;
+
+    let changeset_path = PathBuf::from(CHANGESET_DIR);
+    let changeset = if changeset_path.exists() {
+        ChangeSet::from_directory(&changeset_path)?.into()
+    } else {
+        Vec::new()
+    };
+
+    state.packages = state
+        .packages
+        .into_iter()
+        .map(|package| {
+            prepare_release_for_package(
+                prepare_release,
+                package,
+                &state.all_git_tags,
+                &changeset,
+                state.verbose,
+                &mut dry_run_stdout,
+            )
+        })
+        .try_collect()?;
 
     if let Some(stdout) = dry_run_stdout {
         Ok(RunType::DryRun { state, stdout })
-    } else if !*allow_empty
+    } else if !prepare_release.allow_empty
         && state
             .packages
             .iter()
@@ -103,6 +86,43 @@ pub(crate) fn prepare_release(
     }
 }
 
+fn prepare_release_for_package(
+    prepare_release: &PrepareRelease,
+    mut package: Package,
+    all_tags: &[String],
+    changeset: &[::changesets::Release],
+    verbose: Verbose,
+    dry_run: DryRun,
+) -> Result<Package, Error> {
+    let PrepareRelease {
+        prerelease_label,
+        ignore_conventional_commits,
+        ..
+    } = prepare_release;
+
+    let mut changes = Vec::new();
+    if !ignore_conventional_commits {
+        changes = conventional_commits::get_conventional_commits_after_last_stable_version(
+            &package.name,
+            package.versioning.scopes.as_ref(),
+            &package.versioning.changelog_sections,
+            verbose,
+            all_tags,
+        )?;
+    }
+    changes.extend(changesets::changes_from_changesets(&package, changeset));
+    for change in &changes {
+        if let ChangeSource::ChangeFile(unique_id) = &change.original_source {
+            package.pending_actions.push(Action::RemoveFile {
+                path: RelativePathBuf::from(CHANGESET_DIR).join(unique_id.to_file_name()),
+            });
+        }
+    }
+    package
+        .write_release(&changes, prerelease_label, all_tags, dry_run, verbose)
+        .map_err(Error::from)
+}
+
 pub(crate) fn bump_version(run_type: RunType, rule: &Rule) -> Result<RunType, Error> {
     bump_version_and_update_state(run_type, rule).map_err(Error::from)
 }
@@ -145,6 +165,14 @@ pub(crate) enum Error {
     #[error(transparent)]
     #[diagnostic(transparent)]
     Parse(#[from] changelog::ParseError),
+    #[error(transparent)]
+    #[diagnostic(
+        code(changesets::could_not_read_changeset),
+        help(
+            "This could be a file-system issue or a problem with the formatting of a change file."
+        )
+    )]
+    CouldNotReadChangeSet(#[from] ::changesets::LoadingError),
 }
 
 #[derive(Clone, Debug, Default, Eq, PartialEq)]