From 066ea002a0b8ef63bd91f43f092a16a3ecaaddae Mon Sep 17 00:00:00 2001
From: David Aguilar <davvid@gmail.com>
Date: Sat, 1 Jul 2023 02:36:15 -0700
Subject: [PATCH] model: use Option<&T> instead of &Option<T>

Simplify semantics for callers by always passing Option types directly
instead of passing a reference to an option. This saves a level of
indirection while also making things easier on callers.
---
 src/config/mod.rs          |  2 +-
 src/eval.rs                |  2 +-
 src/model.rs               | 22 +++++++++++-----------
 tests/plant_test.rs        |  2 +-
 tests/query_config_test.rs |  2 +-
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/config/mod.rs b/src/config/mod.rs
index 79784f3..35c3fd6 100644
--- a/src/config/mod.rs
+++ b/src/config/mod.rs
@@ -135,7 +135,7 @@ pub fn read_grafts_recursive(
 
     // Read child grafts recursively after the immutable scope has ended.
     for (graft_name, path, root) in details {
-        app.add_graft_config(id, &graft_name, &path, &root)?;
+        app.add_graft_config(id, &graft_name, &path, root.as_ref())?;
     }
 
     Ok(())
diff --git a/src/eval.rs b/src/eval.rs
index 8c511c4..01f8b16 100644
--- a/src/eval.rs
+++ b/src/eval.rs
@@ -116,7 +116,7 @@ fn expand_vars(
         }
 
         let expr = var.get_expr();
-        let result = value(&app_context, config, expr);
+        let result = value(app_context, config, expr);
         var.set_value(result.clone());
 
         return Some(result);
diff --git a/src/model.rs b/src/model.rs
index ffd5cd9..e55df9e 100644
--- a/src/model.rs
+++ b/src/model.rs
@@ -495,8 +495,8 @@ impl Configuration {
     pub fn update(
         &mut self,
         app_context: &ApplicationContext,
-        config: &Option<std::path::PathBuf>,
-        root: &Option<std::path::PathBuf>,
+        config: Option<&std::path::PathBuf>,
+        root: Option<&std::path::PathBuf>,
         config_verbose: u8,
         parent: Option<ConfigId>,
     ) -> Result<(), errors::GardenError> {
@@ -930,8 +930,8 @@ impl Graft {
         &self.name
     }
 
-    pub fn get_id(&self) -> &Option<ConfigId> {
-        &self.id
+    pub fn get_id(&self) -> Option<ConfigId> {
+        self.id
     }
 
     pub fn set_id(&mut self, id: ConfigId) {
@@ -1214,8 +1214,8 @@ impl ApplicationContext {
         let config_verbose = options.debug_level("config");
         app_context.get_root_config_mut().update(
             &app_context,
-            &options.config,
-            &options.root,
+            options.config.as_ref(),
+            options.root.as_ref(),
             config_verbose,
             None,
         )?;
@@ -1230,14 +1230,14 @@ impl ApplicationContext {
     /// Construct an ApplicationContext from a path and root using default MainOptions.
     pub fn from_path_and_root(
         pathbuf: std::path::PathBuf,
-        root: &Option<std::path::PathBuf>,
+        root: Option<&std::path::PathBuf>,
     ) -> Result<Self, errors::GardenError> {
         let options = cli::MainOptions::new();
         let app_context = Self::new(options.clone());
         let config_verbose = options.debug_level("config");
         app_context.get_root_config_mut().update(
             &app_context,
-            &Some(pathbuf),
+            Some(&pathbuf),
             root,
             config_verbose,
             None,
@@ -1250,7 +1250,7 @@ impl ApplicationContext {
 
     /// Construct an ApplicationContext from a path using default MainOptions.
     pub fn from_path(pathbuf: std::path::PathBuf) -> Result<Self, errors::GardenError> {
-        Self::from_path_and_root(pathbuf, &None)
+        Self::from_path_and_root(pathbuf, None)
     }
 
     /// Construct an ApplicationContext from a path using default MainOptions.
@@ -1306,12 +1306,12 @@ impl ApplicationContext {
         config_id: ConfigId,
         graft_name: &str,
         path: &std::path::Path,
-        root: &Option<std::path::PathBuf>,
+        root: Option<&std::path::PathBuf>,
     ) -> Result<(), errors::GardenError> {
         let path = path.to_path_buf();
         let config_verbose = self.options.debug_level("config");
         let mut graft_config = Configuration::new();
-        graft_config.update(self, &Some(path), root, config_verbose, Some(config_id))?;
+        graft_config.update(self, Some(&path), root, config_verbose, Some(config_id))?;
 
         // The app Arena takes ownershp of the Configuration.
         let graft_id = self.add_graft(config_id, graft_config);
diff --git a/tests/plant_test.rs b/tests/plant_test.rs
index 780d89e..8346c33 100644
--- a/tests/plant_test.rs
+++ b/tests/plant_test.rs
@@ -170,7 +170,7 @@ fn plant_git_worktree() -> Result<()> {
     let pathbuf = fixture.pathbuf("garden.yaml");
     let app_context = garden::model::ApplicationContext::from_path_and_root(
         pathbuf,
-        &Some(fixture.root_pathbuf()),
+        Some(&fixture.root_pathbuf()),
     )?;
     let cfg = app_context.get_root_config();
     assert_eq!(2, cfg.trees.len());
diff --git a/tests/query_config_test.rs b/tests/query_config_test.rs
index 4e9ae92..9dc40b9 100644
--- a/tests/query_config_test.rs
+++ b/tests/query_config_test.rs
@@ -28,7 +28,7 @@ fn tree_name_from_pathbuf() -> Result<()> {
     let pathbuf = std::path::PathBuf::from("tests/data/worktree.yaml");
     let app_context = garden::model::ApplicationContext::from_path_and_root(
         pathbuf,
-        &Some(fixture.root_pathbuf()),
+        Some(&fixture.root_pathbuf()),
     )?;
     let cfg = app_context.get_root_config();