Change --output-proto-directory behavior (#75)

`--output-proto-directory` behavior seems to be not very useful when working with protofetch modules. It changes the default output directory, so it only works if the protofetch.toml file doesn't specify `proto_out_dir`. A more useful behavior seems to be to override the output directory, whether it's set in protofetch.toml or not.

This is technically a breaking change, but it is only breaking if someone has `proto_out_dir` set and is passing a different value to `--output-proto-directory` (which is then ignored).

src/api/builder.rs

@@ -11,11 +11,11 @@ pub struct ProtofetchBuilder {
     module_file_name: Option<PathBuf>,
     lock_file_name: Option<PathBuf>,
     cache_directory_path: Option<PathBuf>,
+    output_directory_name: Option<PathBuf>,
 
     // These fields are deprecated
     http_username: Option<String>,
     http_password: Option<String>,
-    default_output_directory_name: Option<PathBuf>,
     cache_dependencies_directory_name: Option<PathBuf>,
 }
 
@@ -44,17 +44,10 @@ impl ProtofetchBuilder {
         self
     }
 
-    /// Name of the default output directory for proto source files,
-    /// that will be used if `proto_out_dir` is not set in the module toml config.
-    ///
-    /// Defaults to `proto_src`.
-    #[deprecated(
-        since = "0.0.23",
-        note = "overriding the default is not very useful, consider specifying `proto_out_dir` instead"
-    )]
-    #[doc(hidden)]
-    pub fn default_output_directory_name(mut self, path: impl Into<PathBuf>) -> Self {
-        self.default_output_directory_name = Some(path.into());
+    /// Name of the default output directory for proto source files.
+    /// It will override the `proto_out_dir` set in the module toml config.
+    pub fn output_directory_name(mut self, path: impl Into<PathBuf>) -> Self {
+        self.output_directory_name = Some(path.into());
         self
     }
 
@@ -92,7 +85,7 @@ impl ProtofetchBuilder {
             root,
             module_file_name,
             lock_file_name,
-            default_output_directory_name,
+            output_directory_name,
             cache_directory_path,
             http_username,
             http_password,
@@ -107,9 +100,6 @@ impl ProtofetchBuilder {
 
         let lock_file_name = lock_file_name.unwrap_or_else(|| PathBuf::from("protofetch.lock"));
 
-        let default_output_directory_name =
-            default_output_directory_name.unwrap_or_else(|| PathBuf::from("proto_src"));
-
         let cache_directory =
             root.join(cache_directory_path.unwrap_or_else(default_cache_directory));
 
@@ -128,7 +118,7 @@ impl ProtofetchBuilder {
             root,
             module_file_name,
             lock_file_name,
-            default_output_directory_name,
+            output_directory_name,
             cache_dependencies_directory_name,
         })
     }

src/api/mod.rs

@@ -17,7 +17,7 @@ pub struct Protofetch {
     root: PathBuf,
     module_file_name: PathBuf,
     lock_file_name: PathBuf,
-    default_output_directory_name: PathBuf,
+    output_directory_name: Option<PathBuf>,
     cache_dependencies_directory_name: PathBuf,
 }
 
@@ -40,7 +40,7 @@ impl Protofetch {
             &self.module_file_name,
             &self.lock_file_name,
             &self.cache_dependencies_directory_name,
-            &self.default_output_directory_name,
+            self.output_directory_name.as_deref(),
         )
     }
 
@@ -74,7 +74,7 @@ impl Protofetch {
         do_clean(
             &self.root,
             &self.lock_file_name,
-            &self.default_output_directory_name,
+            self.output_directory_name.as_deref(),
         )
     }
 

src/cli/command_handlers.rs

@@ -14,6 +14,8 @@ use std::{
     path::{Path, PathBuf},
 };
 
+const DEFAULT_OUTPUT_DIRECTORY_NAME: &str = "proto_src";
+
 /// Handler to fetch command
 pub fn do_fetch(
     force_lock: bool,
@@ -22,7 +24,7 @@ pub fn do_fetch(
     module_file_name: &Path,
     lock_file_name: &Path,
     cache_dependencies_directory_name: &Path,
-    default_proto_output_directory_name: &Path,
+    output_directory_name: Option<&Path>,
 ) -> Result<(), Box<dyn Error>> {
     let lock_file_path = root.join(lock_file_name);
     let lockfile = if force_lock || !lock_file_path.exists() {
@@ -31,16 +33,14 @@ pub fn do_fetch(
         LockFile::from_file(&lock_file_path)?
     };
     let cache_dependencies_directory_path = cache.location.join(cache_dependencies_directory_name);
-    let proto_output_directory_name = lockfile
-        .proto_out_dir
-        .as_ref()
-        .map(Path::new)
-        .unwrap_or(default_proto_output_directory_name);
-    let proto_output_directory_path = root.join(proto_output_directory_name);
+    let output_directory_name = output_directory_name
+        .or_else(|| lockfile.proto_out_dir.as_ref().map(Path::new))
+        .unwrap_or(Path::new(DEFAULT_OUTPUT_DIRECTORY_NAME));
+    let output_directory_path = root.join(output_directory_name);
     fetch::fetch_sources(cache, &lockfile, &cache_dependencies_directory_path)?;
     //Copy proto_out files to actual target
     proto::copy_proto_files(
-        &proto_output_directory_path,
+        &output_directory_path,
         &cache_dependencies_directory_path,
         &lockfile,
     )?;
@@ -120,22 +120,20 @@ pub fn do_migrate(
 pub fn do_clean(
     root: &Path,
     lock_file_name: &Path,
-    default_output_directory_name: &Path,
+    output_directory_name: Option<&Path>,
 ) -> Result<(), Box<dyn Error>> {
     let lock_file_path = root.join(lock_file_name);
     if lock_file_path.exists() {
         let lockfile = LockFile::from_file(&lock_file_path)?;
-        let proto_out_directory_name = lockfile
-            .proto_out_dir
-            .as_ref()
-            .map(Path::new)
-            .unwrap_or(default_output_directory_name);
-        let proto_out_directory_path = root.join(proto_out_directory_name);
+        let output_directory_name = output_directory_name
+            .or_else(|| lockfile.proto_out_dir.as_ref().map(Path::new))
+            .unwrap_or(Path::new(DEFAULT_OUTPUT_DIRECTORY_NAME));
+        let output_directory_path = root.join(output_directory_name);
         info!(
             "Cleaning protofetch proto_out source files folder {}.",
-            proto_out_directory_path.display()
+            output_directory_path.display()
         );
-        std::fs::remove_dir_all(proto_out_directory_path)?;
+        std::fs::remove_dir_all(output_directory_path)?;
         std::fs::remove_file(lock_file_path)?;
         Ok(())
     } else {

src/main.rs

@@ -21,9 +21,9 @@ pub struct CliArgs {
     /// Location of the protofetch cache directory [default: platform-specific]
     pub cache_directory: Option<String>,
     /// Name of the output directory for proto source files,
-    /// this will be used if parameter proto_out_dir is not present in the module toml config
-    #[clap(short, long, default_value = "proto_src")]
-    pub output_proto_directory: String,
+    /// this will override proto_out_dir from the module toml config
+    #[clap(short, long)]
+    pub output_proto_directory: Option<String>,
     #[clap(short, long)]
     /// Git username in case https is used in config
     pub username: Option<String>,
@@ -84,9 +84,11 @@ fn run() -> Result<(), Box<dyn Error>> {
     let mut protofetch = Protofetch::builder()
         .module_file_name(&cli_args.module_location)
         .lock_file_name(&cli_args.lockfile_location)
-        .default_output_directory_name(&cli_args.output_proto_directory)
         .http_credentials(cli_args.username, cli_args.password);
 
+    if let Some(output_directory_name) = &cli_args.output_proto_directory {
+        protofetch = protofetch.output_directory_name(output_directory_name)
+    }
     if let Some(cache_directory) = &cli_args.cache_directory {
         protofetch = protofetch.cache_directory(cache_directory);
     }