Add skipped option to ActionState (#74)

* Make plans versioned

* Delint

* speeeeeeeeling

* remove file that was dead

* Flesh out docs and improve public API

* Speeling

* Fixups

* Fix doctests

* Do a better job with actionstate

* Add some more docs

* Fix doctest

* Make CLI stuff optional

* Touchup

* Speeling

* add skipped to ActionState

src/action/base/create_directory.rs

@@ -49,7 +49,7 @@ impl CreateDirectory {
                     path.display(),
                 );
                 // TODO: Validate owner/group...
-                ActionState::Completed
+                ActionState::Skipped
             } else {
                 return Err(CreateDirectoryError::Exists(std::io::Error::new(
                     std::io::ErrorKind::AlreadyExists,

src/action/darwin/create_apfs_volume.rs

@@ -6,7 +6,7 @@ use crate::action::StatefulAction;
 use crate::execute_command;
 
 use crate::{
-    action::{Action, ActionDescription, ActionState},
+    action::{Action, ActionDescription},
     BoxableError,
 };
 
@@ -15,7 +15,6 @@ pub struct CreateApfsVolume {
     disk: PathBuf,
     name: String,
     case_sensitive: bool,
-    action_state: ActionState,
 }
 
 impl CreateApfsVolume {
@@ -29,7 +28,6 @@ impl CreateApfsVolume {
             disk: disk.as_ref().to_path_buf(),
             name,
             case_sensitive,
-            action_state: ActionState::Uncompleted,
         }
         .into())
     }
@@ -60,7 +58,6 @@ impl Action for CreateApfsVolume {
             disk,
             name,
             case_sensitive,
-            action_state: _,
         } = self;
 
         execute_command(
@@ -107,7 +104,6 @@ impl Action for CreateApfsVolume {
             disk: _,
             name,
             case_sensitive: _,
-            action_state: _,
         } = self;
 
         execute_command(

src/action/stateful.rs

@@ -25,53 +25,73 @@ where
 impl StatefulAction<Box<dyn Action>> {
     /// A description of what this action would do during execution
     pub fn describe_execute(&self) -> Vec<ActionDescription> {
-        if self.state == ActionState::Completed {
+        match self.state {
-            return vec![];
+            ActionState::Uncompleted | ActionState::Skipped => {
+                vec![]
+            },
+            _ => self.action.execute_description(),
         }
-        return self.action.execute_description();
     }
     /// A description of what this action would do during revert
     pub fn describe_revert(&self) -> Vec<ActionDescription> {
-        if self.state == ActionState::Uncompleted {
+        match self.state {
-            return vec![];
+            ActionState::Completed | ActionState::Skipped => {
+                vec![]
+            },
+            _ => self.action.revert_description(),
         }
-        return self.action.revert_description();
     }
     /// Perform any execution steps
     ///
     /// You should prefer this ([`try_execute`][StatefulAction::try_execute]) over [`execute`][Action::execute] as it handles [`ActionState`] and does tracing
     pub async fn try_execute(&mut self) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
-        if self.state == ActionState::Completed {
+        match self.state {
-            tracing::trace!(
+            ActionState::Completed => {
-                "Completed: (Already done) {}",
+                tracing::trace!(
-                self.action.tracing_synopsis()
+                    "Completed: (Already done) {}",
-            );
+                    self.action.tracing_synopsis()
-            return Ok(());
+                );
+                Ok(())
+            },
+            ActionState::Skipped => {
+                tracing::trace!("Skipped: {}", self.action.tracing_synopsis());
+                Ok(())
+            },
+            _ => {
+                self.state = ActionState::Progress;
+                tracing::debug!("Executing: {}", self.action.tracing_synopsis());
+                self.action.execute().await?;
+                self.state = ActionState::Completed;
+                tracing::debug!("Completed: {}", self.action.tracing_synopsis());
+                Ok(())
+            },
         }
-        self.state = ActionState::Progress;
-        tracing::debug!("Executing: {}", self.action.tracing_synopsis());
-        self.action.execute().await?;
-        self.state = ActionState::Completed;
-        tracing::debug!("Completed: {}", self.action.tracing_synopsis());
-        Ok(())
     }
     /// Perform any revert steps
     ///
     /// You should prefer this ([`try_revert`][StatefulAction::try_revert]) over [`revert`][Action::revert] as it handles [`ActionState`] and does tracing
     pub async fn try_revert(&mut self) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
-        if self.state == ActionState::Uncompleted {
+        match self.state {
-            tracing::trace!(
+            ActionState::Uncompleted => {
-                "Reverted: (Already done) {}",
+                tracing::trace!(
-                self.action.tracing_synopsis()
+                    "Reverted: (Already done) {}",
-            );
+                    self.action.tracing_synopsis()
-            return Ok(());
+                );
+                Ok(())
+            },
+            ActionState::Skipped => {
+                tracing::trace!("Skipped: {}", self.action.tracing_synopsis());
+                Ok(())
+            },
+            _ => {
+                self.state = ActionState::Progress;
+                tracing::debug!("Reverting: {}", self.action.tracing_synopsis());
+                self.action.revert().await?;
+                tracing::debug!("Reverted: {}", self.action.tracing_synopsis());
+                self.state = ActionState::Uncompleted;
+                Ok(())
+            },
         }
-        self.state = ActionState::Progress;
-        tracing::debug!("Reverting: {}", self.action.tracing_synopsis());
-        self.action.revert().await?;
-        tracing::debug!("Reverted: {}", self.action.tracing_synopsis());
-        self.state = ActionState::Uncompleted;
-        Ok(())
     }
 }
 
@@ -110,37 +130,53 @@ where
     ///
     /// You should prefer this ([`try_execute`][StatefulAction::try_execute]) over [`execute`][Action::execute] as it handles [`ActionState`] and does tracing
     pub async fn try_execute(&mut self) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
-        if self.state == ActionState::Completed {
+        match self.state {
-            tracing::trace!(
+            ActionState::Completed => {
-                "Completed: (Already done) {}",
+                tracing::trace!(
-                self.action.tracing_synopsis()
+                    "Completed: (Already done) {}",
-            );
+                    self.action.tracing_synopsis()
-            return Ok(());
+                );
+                Ok(())
+            },
+            ActionState::Skipped => {
+                tracing::trace!("Skipped: {}", self.action.tracing_synopsis());
+                Ok(())
+            },
+            _ => {
+                self.state = ActionState::Progress;
+                tracing::debug!("Executing: {}", self.action.tracing_synopsis());
+                self.action.execute().await?;
+                self.state = ActionState::Completed;
+                tracing::debug!("Completed: {}", self.action.tracing_synopsis());
+                Ok(())
+            },
         }
-        self.state = ActionState::Progress;
-        tracing::debug!("Executing: {}", self.action.tracing_synopsis());
-        self.action.execute().await?;
-        self.state = ActionState::Completed;
-        tracing::debug!("Completed: {}", self.action.tracing_synopsis());
-        Ok(())
     }
     /// Perform any revert steps
     ///
     /// You should prefer this ([`try_revert`][StatefulAction::try_revert]) over [`revert`][Action::revert] as it handles [`ActionState`] and does tracing
     pub async fn try_revert(&mut self) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
-        if self.state == ActionState::Uncompleted {
+        match self.state {
-            tracing::trace!(
+            ActionState::Uncompleted => {
-                "Reverted: (Already done) {}",
+                tracing::trace!(
-                self.action.tracing_synopsis()
+                    "Reverted: (Already done) {}",
-            );
+                    self.action.tracing_synopsis()
-            return Ok(());
+                );
+                Ok(())
+            },
+            ActionState::Skipped => {
+                tracing::trace!("Skipped: {}", self.action.tracing_synopsis());
+                Ok(())
+            },
+            _ => {
+                self.state = ActionState::Progress;
+                tracing::debug!("Reverting: {}", self.action.tracing_synopsis());
+                self.action.revert().await?;
+                tracing::debug!("Reverted: {}", self.action.tracing_synopsis());
+                self.state = ActionState::Uncompleted;
+                Ok(())
+            },
         }
-        self.state = ActionState::Progress;
-        tracing::debug!("Reverting: {}", self.action.tracing_synopsis());
-        self.action.revert().await?;
-        tracing::debug!("Reverted: {}", self.action.tracing_synopsis());
-        self.state = ActionState::Uncompleted;
-        Ok(())
     }
 }
 
@@ -165,4 +201,11 @@ pub enum ActionState {
     on [`InstallPlan::uninstall`](crate::InstallPlan::uninstall) and executed on [`InstallPlan::install`](crate::InstallPlan::install)
     */
     Uncompleted,
+    /**
+    If [`Skipped`](ActionState::Skipped) an [`Action`](crate::action::Action) will be skipped
+    on [`InstallPlan::install`](crate::InstallPlan::install) and [`InstallPlan::uninstall`](crate::InstallPlan::uninstall)
+
+    Typically this is used by actions which detect they are already completed in their `plan` phase.
+    */
+    Skipped,
 }