Merge pull request 'item id arggroup' (#28) from item-number-refactoring into main

Reviewed-on: #28

src/main.rs

@@ -7,7 +7,7 @@ mod git;
 
 use storage::{Storage, Item};
 
-use clap::{Parser, Subcommand};
+use clap::{Parser, Subcommand, ArgGroup};
 use rpassword;
 
 use std::io::{self, Write};
@@ -34,15 +34,7 @@ enum Commands {
     List,
 
     /// Show content of an item
-    Show {
-        // positional
-        #[arg(value_name="item_id", required=false)]
-        id: Option<String>,
-        
-        // named
-        #[arg(short, long, value_name="item_number", required=false)]
-        number: Option<u32>
-    },
+    Show(ItemIdArgs),
     
     /// Adds new item with unique id to the storage
     Add {
@@ -51,23 +43,28 @@ enum Commands {
     },
 
     /// Edit item content
-    Edit {
-        // positional
-        #[arg(value_name="item_id", required=false)]
-        id: Option<String>,
-
-        // named
-        #[arg(short, long, value_name="item_number", required=false)]
-        number: Option<u32>
-    },
+    Edit(ItemIdArgs),
 
-    Delete {
-        #[arg(value_name="item_id")]
-        id: String
-    }
+    /// Delete item
+    Delete(ItemIdArgs)
 
 }
 
+#[derive(Parser)]
+#[command(group(
+    ArgGroup::new("item")
+        .required(true)
+        .args(&["id", "number"])
+))]
+struct ItemIdArgs {
+    /// Item id
+    #[arg(required=false)]
+    id: Option<String>,
+    
+    #[arg(short='n', long, value_name="number")]
+    number: Option<u32>
+}
+
 enum PROMPT {
     YES,
     NO
@@ -257,6 +254,13 @@ impl MPS {
         Ok(())
     }
 
+    fn delete_by_number(&mut self, number: u32) -> io::Result<()> {
+        self.login()?;
+        let st = self.storage.as_mut().unwrap();
+        let id = st.get_id_by_number(number)?;
+        self.delete(&id)
+    }
+
 }
 
 fn run_command() -> io::Result<()> {
@@ -269,61 +273,36 @@ fn run_command() -> io::Result<()> {
             let mut mps = MPS::new();
             mps.list()?;
         }
-        Some(Commands::Show { id, number }) => {
-            match (id, number) {
-                (None, None) => {
-                    return Err(io::Error::new(
-                        io::ErrorKind::InvalidInput,
-                        "Must provide <item_id> or -n <item_number>"
-                    ));
-                }
-                (Some(item_id), None) => {
-                    let mut mps = MPS::new();
-                    mps.show(item_id)?;
-                },
-                (None, Some(item_number)) => {
-                    let mut mps = MPS::new();
-                    mps.show_by_number(*item_number)?;
-                },
-                _ => {
-                    return Err(io::Error::new(
-                        io::ErrorKind::InvalidInput,
-                        "Must provide <item_id> or -n <item_number> but not both"
-                    ))
-                }
-            };
+        Some(Commands::Show(item)) => {
+            let mut mps = MPS::new();
+            if let Some(id) = &item.id {
+                mps.show(id)?;
+            }
+            if let Some(number) = &item.number {
+                mps.show_by_number(*number)?;
+            }
         }
         Some(Commands::Add { id }) => {
             let mut mps = MPS::new();
             mps.add(id)?;
         }
-        Some(Commands::Edit { id, number }) => {
-            match (id, number) {
-                (None, None) => {
-                    return Err(io::Error::new(
-                        io::ErrorKind::InvalidInput,
-                        "Must provide <item_id> or -n <item_number>"
-                    ));
-                }
-                (Some(item_id), None) => {
-                    let mut mps = MPS::new();
-                    mps.edit(item_id)?;
-                },
-                (None, Some(item_number)) => {
-                    let mut mps = MPS::new();
-                    mps.edit_by_number(*item_number)?;
-                },
-                _ => {
-                    return Err(io::Error::new(
-                        io::ErrorKind::InvalidInput,
-                        "Must provide <item_id> or -n <item_number> but not both"
-                    ))
-                }
-            };
+        Some(Commands::Edit(item)) => {
+            let mut mps = MPS::new();
+            if let Some(id) = &item.id {
+                mps.edit(id)?;
+            }
+            if let Some(number) = &item.number {
+                mps.edit_by_number(*number)?;
+            }
         }
-        Some(Commands::Delete { id }) => {
+        Some(Commands::Delete(item)) => {
             let mut mps = MPS::new();
-            mps.delete(id)?
+            if let Some(id) = &item.id {
+                mps.delete(id)?
+            }
+            if let Some(number) = &item.number {
+                mps.delete_by_number(*number)?;
+            }
         }
         None => {
             match Storage::check_installed() {

src/storage.rs

@@ -197,7 +197,7 @@ impl Storage {
             ));
         }
         let ids = self.ids();
-        if number >= ids.len() {
+        if number > ids.len() {
             return Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
                 format!("There are only {} items, but asked with number {}", ids.len(), number)