From 7c8ee7a50044954f15eef973c3bd873eaebbf93a Mon Sep 17 00:00:00 2001 From: qwjyh Date: Tue, 12 Mar 2024 16:18:24 +0900 Subject: [PATCH] redesign storage add Commands - replace storage add args with subcommands of physical, directory, online - to make argument dependencies clearer --- README.md | 11 +- src/cmd_args.rs | 59 +++++-- src/cmd_init.rs | 4 + src/cmd_storage.rs | 196 ++++++++++------------- src/main.rs | 6 +- src/storages/directory.rs | 7 +- src/storages/online_storage.rs | 6 + src/storages/physical_drive_partition.rs | 22 +-- 8 files changed, 169 insertions(+), 142 deletions(-) diff --git a/README.md b/README.md index 5dd65e9..a1ebe8c 100644 --- a/README.md +++ b/README.md @@ -1,14 +1,19 @@ # TODO: - [x] split subcommands to functions - [x] write test for init subcommand - - [ ] write test with existing repo + - [x] write test with existing repo - [x] with ssh credential - [x] ssh-agent - [x] specify key +- [ ] write test for storage subcommand + - [ ] storage add online + - [ ] storage add directory + - [ ] storage list - [ ] add storage remove command - [ ] add sync subcommand - [ ] add check subcommand -- [ ] reorganize cmd option for storage - - [ ] use subcommand +- [x] reorganize cmd option for storage + - [x] use subcommand +- [ ] no commit option diff --git a/src/cmd_args.rs b/src/cmd_args.rs index cfc0659..d1bbaaf 100644 --- a/src/cmd_args.rs +++ b/src/cmd_args.rs @@ -3,6 +3,7 @@ use crate::StorageType; use crate::path; use crate::PathBuf; +use clap::Args; use clap::{Parser, Subcommand}; use clap_verbosity_flag::Verbosity; @@ -50,7 +51,7 @@ pub(crate) enum Commands { Check {}, } -#[derive(clap::Args, Debug)] +#[derive(Args, Debug)] #[command(args_conflicts_with_subcommands = true)] pub(crate) struct StorageArgs { #[command(subcommand)] @@ -60,14 +61,7 @@ pub(crate) struct StorageArgs { #[derive(Subcommand, Debug)] pub(crate) enum StorageCommands { /// Add new storage. - Add { - #[arg(value_enum)] - storage_type: StorageType, - - // TODO: set this require and select matching disk for physical - #[arg(short, long, value_name = "PATH")] - path: Option, - }, + Add(StorageAddArgs), /// List all storages. List { /// Show note on the storages. @@ -87,3 +81,50 @@ pub(crate) enum StorageCommands { path: path::PathBuf, }, } + +#[derive(Args, Debug)] +pub(crate) struct StorageAddArgs { + #[command(subcommand)] + pub(crate) command: StorageAddCommands, +} + +#[derive(Subcommand, Debug)] +pub(crate) enum StorageAddCommands { + /// Physical drive partition. + Physical { + /// Unique name for the storage. + name: String, + /// Path where the storage is mounted on this device. + /// leave blank to fetch system info automatically. + path: Option, + }, + /// Sub directory of other storages. + Directory { + /// Unique name for the storage. + name: String, + /// Path where the storage is mounted on this device. + path: PathBuf, + /// Additional info. Empty by default. + #[arg(short, long, default_value = "")] + notes: String, + /// Device specific alias for the storage. + #[arg(short, long)] + alias: String, + }, + /// Online storage. + Online { + /// Unique name for the storage. + name: String, + /// Path where the storage is mounted on this device. + path: PathBuf, + /// Provider name (for the common information). + #[arg(short, long)] + provider: String, + /// Capacity in bytes. + #[arg(short, long)] + capacity: u64, + /// Device specific alias for the storage. + #[arg(short, long)] + alias: String, + } +} diff --git a/src/cmd_init.rs b/src/cmd_init.rs index f5552d4..debec88 100644 --- a/src/cmd_init.rs +++ b/src/cmd_init.rs @@ -76,6 +76,10 @@ pub(crate) fn cmd_init( ssh_key: Option, config_dir: &path::PathBuf, ) -> Result<()> { + if config_dir.join(DEVICESFILE).exists() { + debug!("{} already exists.", DEVICESFILE); + return Err(anyhow!("This device is already added.")); + } // validate device name if device_name.chars().count() == 0 { log::error!("Device name cannnot by empty"); diff --git a/src/cmd_storage.rs b/src/cmd_storage.rs index 2ce9e31..d37a8da 100644 --- a/src/cmd_storage.rs +++ b/src/cmd_storage.rs @@ -16,149 +16,84 @@ use unicode_width::{self, UnicodeWidthStr}; use crate::{ add_and_commit, - cmd_args::Cli, + cmd_args::{Cli, StorageAddCommands}, devices::{self, Device}, inquire_filepath_completer::FilePathCompleter, storages::{ - self, directory, local_info, physical_drive_partition, Storage, StorageExt, StorageType, - Storages, + self, directory, local_info, + physical_drive_partition::{self, PhysicalDrivePartition}, + Storage, StorageExt, StorageType, Storages, }, }; pub(crate) fn cmd_storage_add( - storage_type: storages::StorageType, - path: Option, + args: StorageAddCommands, repo: Repository, config_dir: &PathBuf, ) -> Result<()> { - trace!("Storage Add {:?}, {:?}", storage_type, path); + trace!("Storage Add with args: {:?}", args); // Get storages let mut storages = Storages::read(&config_dir)?; trace!("found storages: {:?}", storages); let device = devices::get_device(&config_dir)?; - let storage = match storage_type { - StorageType::P => { - let use_sysinfo = { - let options = vec![ - "Fetch disk information automatically.", - "Type disk information manually.", - ]; - let ans = Select::new( - "Do you fetch disk information automatically? (it may take a few minutes)", - options, - ) - .prompt() - .context("Failed to get response. Please try again.")?; - match ans { - "Fetch disk information automatically." => true, - _ => false, - } - }; + let storage = match args { + StorageAddCommands::Physical { name, path } => { + if !is_unique_name(&name, &storages) { + return Err(anyhow!( + "The name {} is already used for another storage.", + name + )); + } + let use_sysinfo = path.is_none(); let storage = if use_sysinfo { - // select storage - physical_drive_partition::select_physical_storage(device, &storages)? + physical_drive_partition::select_physical_storage(name, device)? } else { - let mut name = String::new(); - loop { - name = Text::new("Name for the storage:") - .with_validator(min_length!(0, "At least 1 character")) - .prompt() - .context("Failed to get Name")?; - if storages.list.iter().all(|(k, _v)| k != &name) { - break; - } - println!("The name {} is already used.", name); - } - let kind = Text::new("Kind of storage (ex. SSD):") - .prompt() - .context("Failed to get kind.")?; - let capacity: u64 = CustomType::::new("Capacity (byte):") - .with_error_message("Please type number.") - .prompt() - .context("Failed to get capacity.")?; - let fs = Text::new("filesystem:") - .prompt() - .context("Failed to get fs.")?; - let is_removable = Confirm::new("Is removable") - .prompt() - .context("Failed to get is_removable")?; - let mount_path: PathBuf = PathBuf::from( - Text::new("mount path:") - .with_autocomplete(FilePathCompleter::default()) - .prompt()?, - ); - let local_info = local_info::LocalInfo::new("".to_string(), mount_path); - physical_drive_partition::PhysicalDrivePartition::new( - name, - kind, - capacity, - fs, - is_removable, - local_info, - &device, - ) + manually_construct_physical_drive_partition(name, path.unwrap(), &device)? }; println!("storage: {}: {:?}", storage.name(), storage); Storage::PhysicalStorage(storage) } - StorageType::S => { + StorageAddCommands::Directory { + name, + path, + notes, + alias, + } => { + if !is_unique_name(&name, &storages) { + return Err(anyhow!( + "The name {} is already used for another storage.", + name + )); + } if storages.list.is_empty() { return Err(anyhow!( "No storages found. Please add at least 1 physical/online storage first to add sub directory." )); } - let path = path.unwrap_or_else(|| { - let mut cmd = Cli::command(); - // TODO: weired def of cmd argument - cmd.error( - ErrorKind::MissingRequiredArgument, - " is required with sub-directory", - ) - .exit(); - }); trace!("SubDirectory arguments: path: {:?}", path); // Nightly feature std::path::absolute let path = path.canonicalize()?; trace!("canonicalized: path: {:?}", path); - let key_name = ask_unique_name(&storages, "sub-directory".to_string())?; - let notes = Text::new("Notes for this sub-directory:").prompt()?; let storage = directory::Directory::try_from_device_path( - key_name, path, notes, &device, &storages, + name, path, notes, alias, &device, &storages, )?; Storage::SubDirectory(storage) } - StorageType::O => { - let path = path.unwrap_or_else(|| { - let mut cmd = Cli::command(); - cmd.error( - ErrorKind::MissingRequiredArgument, - " is required with sub-directory", - ) - .exit(); - }); - let mut name = String::new(); - loop { - name = Text::new("Name for the storage:") - .with_validator(min_length!(0, "At least 1 character")) - .prompt() - .context("Failed to get Name")?; - if storages.list.iter().all(|(k, _v)| k != &name) { - break; - } - println!("The name {} is already used.", name); + StorageAddCommands::Online { + name, + path, + provider, + capacity, + alias, + } => { + if !is_unique_name(&name, &storages) { + return Err(anyhow!( + "The name {} is already used for another storage.", + name + )); } - let provider = Text::new("Provider:") - .prompt() - .context("Failed to get provider")?; - let capacity: u64 = CustomType::::new("Capacity (byte):") - .with_error_message("Please type number.") - .prompt() - .context("Failed to get capacity.")?; - let alias = Text::new("Alias:") - .prompt() - .context("Failed to get provider")?; let storage = storages::online_storage::OnlineStorage::new( name, provider, capacity, alias, path, &device, ); @@ -168,6 +103,7 @@ pub(crate) fn cmd_storage_add( // add to storages let new_storage_name = storage.name().clone(); + let new_storage_type = storage.typename().to_string(); storages.add(storage)?; trace!("updated storages: {:?}", storages); @@ -178,7 +114,10 @@ pub(crate) fn cmd_storage_add( add_and_commit( &repo, &Path::new(storages::STORAGESFILE), - &format!("Add new storage(physical drive): {}", new_storage_name), + &format!( + "Add new storage({}): {}", + new_storage_type, new_storage_name + ), )?; println!("Added new storage."); @@ -186,10 +125,51 @@ pub(crate) fn cmd_storage_add( Ok(()) } +fn is_unique_name(newname: &String, storages: &Storages) -> bool { + storages.list.iter().all(|(name, _)| name != newname) +} + +fn manually_construct_physical_drive_partition( + name: String, + path: PathBuf, + device: &Device, +) -> Result { + let kind = Text::new("Kind of storage (ex. SSD):") + .prompt() + .context("Failed to get kind.")?; + let capacity: u64 = CustomType::::new("Capacity (byte):") + .with_error_message("Please type number.") + .prompt() + .context("Failed to get capacity.")?; + let fs = Text::new("filesystem:") + .prompt() + .context("Failed to get fs.")?; + let is_removable = Confirm::new("Is removable") + .prompt() + .context("Failed to get is_removable")?; + let alias = Text::new("Alias of the storage for this device") + .prompt() + .context("Failed to get alias.")?; + let local_info = local_info::LocalInfo::new(alias, path); + Ok(physical_drive_partition::PhysicalDrivePartition::new( + name, + kind, + capacity, + fs, + is_removable, + local_info, + &device, + )) +} + pub(crate) fn cmd_storage_list(config_dir: &PathBuf, with_note: bool) -> Result<()> { // Get storages let storages = Storages::read(&config_dir)?; trace!("found storages: {:?}", storages); + if storages.list.is_empty() { + println!("No storages found"); + return Ok(()); + } let device = devices::get_device(&config_dir)?; let mut stdout = io::BufWriter::new(io::stdout()); write_storages_list(&mut stdout, &storages, &device, with_note)?; diff --git a/src/main.rs b/src/main.rs index 199af0f..ede7776 100644 --- a/src/main.rs +++ b/src/main.rs @@ -71,9 +71,9 @@ fn main() -> Result<()> { )?; trace!("repo state: {:?}", repo.state()); match storage.command { - StorageCommands::Add { storage_type, path } => { - cmd_storage::cmd_storage_add(storage_type, path, repo, &config_dir)? - } + StorageCommands::Add(storageargs) => { + cmd_storage::cmd_storage_add(storageargs.command, repo, &config_dir)? + }, StorageCommands::List { long } => cmd_storage::cmd_storage_list(&config_dir, long)?, StorageCommands::Bind { storage: storage_name, diff --git a/src/storages/directory.rs b/src/storages/directory.rs index b0aca20..d9fc716 100644 --- a/src/storages/directory.rs +++ b/src/storages/directory.rs @@ -15,10 +15,14 @@ use super::{local_info::LocalInfo, Storage, StorageExt, Storages}; /// Subdirectory of other [Storage]s. #[derive(Serialize, Deserialize, Debug)] pub struct Directory { + /// ID. name: String, + /// ID of parent storage. parent: String, + /// Relative path to the parent storage. relative_path: path::PathBuf, pub notes: String, + /// Device and localinfo pairs. local_infos: HashMap, } @@ -47,6 +51,7 @@ impl Directory { name: String, path: path::PathBuf, notes: String, + alias: String, device: &devices::Device, storages: &Storages, ) -> Result { @@ -69,7 +74,7 @@ impl Directory { }) .context(format!("Failed to compare diff of paths"))?; trace!("Selected parent: {}", parent_name); - let local_info = LocalInfo::new("".to_string(), path); + let local_info = LocalInfo::new(alias, path); Ok(Directory::new( name, parent_name.clone(), diff --git a/src/storages/online_storage.rs b/src/storages/online_storage.rs index b6b6203..ff70bf2 100644 --- a/src/storages/online_storage.rs +++ b/src/storages/online_storage.rs @@ -16,13 +16,19 @@ use super::{ #[derive(Serialize, Deserialize, Debug)] pub struct OnlineStorage { + /// ID. name: String, + /// Provider string (for the common information). pub provider: String, + /// Capacity in bytes. capacity: u64, + /// Device and local info pairs. local_infos: HashMap, } impl OnlineStorage { + /// # Arguments + /// - alias: for [`LocalInfo`] pub fn new( name: String, provider: String, diff --git a/src/storages/physical_drive_partition.rs b/src/storages/physical_drive_partition.rs index 51adc75..826efc4 100644 --- a/src/storages/physical_drive_partition.rs +++ b/src/storages/physical_drive_partition.rs @@ -7,11 +7,8 @@ use anyhow::{anyhow, Context, Result}; use byte_unit::Byte; use inquire::Text; use serde::{Deserialize, Serialize}; -use std::path::{self, PathBuf}; -use std::{ - collections::{hash_map::RandomState, HashMap}, - fmt, -}; +use std::path; +use std::{collections::HashMap, fmt}; use sysinfo::{Disk, DiskExt, SystemExt}; use super::local_info::{self, LocalInfo}; @@ -191,8 +188,8 @@ impl fmt::Display for PhysicalDrivePartition { /// Interactively select physical storage from available disks in sysinfo. pub fn select_physical_storage( + disk_name: String, device: Device, - storages: &Storages, ) -> Result { trace!("select_physical_storage"); // get disk info fron sysinfo @@ -206,22 +203,11 @@ pub fn select_physical_storage( trace!("{:?}", disk) } let disk = select_sysinfo_disk(&sys_disks)?; - // name the disk - let mut disk_name = String::new(); - trace!("{}", disk_name); - loop { - disk_name = Text::new("Name for the disk:").prompt()?; - if storages.list.iter().all(|(k, v)| k != &disk_name) { - break; - } - println!("The name {} is already used.", disk_name); - } - trace!("selected name: {}", disk_name); let storage = PhysicalDrivePartition::try_from_sysinfo_disk(&disk, disk_name, device)?; Ok(storage) } -pub fn select_sysinfo_disk(sysinfo: &sysinfo::System) -> Result<&Disk> { +fn select_sysinfo_disk(sysinfo: &sysinfo::System) -> Result<&Disk> { let available_disks = sysinfo .disks() .iter()