Refactor mountable devices display

7 files changed, 100 insertions, 42 deletions

diff --git a/development.md b/development.md
index dd3c7c6..648cc1c 100644
--- a/development.md
+++ b/development.md
@@ -632,6 +632,7 @@ New view: Tree ! Toggle with 't', fold with 'z'. Navigate normally.
   - [ ] Draw trait
 - [x] better messages when asking a password
 - [x] FIX: trash is buggy. Can't delete definitely. Display is wrong when empty.
+- [ ] FIX: cursor is off by one in password
 
 ## TODO
 
diff --git a/src/cryptsetup.rs b/src/cryptsetup.rs
index 261fa2a..0e97fa4 100644
--- a/src/cryptsetup.rs
+++ b/src/cryptsetup.rs
@@ -279,6 +279,10 @@ impl MountHelper for CryptoDevice {
             format!("{} - not mounted", self.path)
         })
     }
+
+    fn device_name(&self) -> Result<String> {
+        self.as_string()
+    }
 }
 
 /// Holds the device itself and its passwords.
diff --git a/src/iso.rs b/src/iso.rs
index 575e3e6..ae0f07d 100644
--- a/src/iso.rs
+++ b/src/iso.rs
@@ -146,4 +146,8 @@ impl MountHelper for IsoDevice {
             Ok(format!("not mounted {}", self.path))
         }
     }
+
+    fn device_name(&self) -> Result<String> {
+        Ok(self.path.to_owned())
+    }
 }
diff --git a/src/mount_help.rs b/src/mount_help.rs
index fc50f83..9155d9a 100644
--- a/src/mount_help.rs
+++ b/src/mount_help.rs
@@ -24,4 +24,6 @@ pub trait MountHelper {
 
     /// String representation of the device
     fn as_string(&self) -> Result<String>;
+
+    fn device_name(&self) -> Result<String>;
 }
diff --git a/src/removable_devices.rs b/src/removable_devices.rs
index 7759c83..9e702fd 100644
--- a/src/removable_devices.rs
+++ b/src/removable_devices.rs
@@ -3,6 +3,8 @@ use anyhow::{anyhow, Result};
 use crate::{
     constant_strings_paths::GIO,
     impl_selectable_content,
+    mount_help::MountHelper,
+    password::PasswordHolder,
     utils::{current_uid, is_dir_empty, is_program_in_path},
 };
 
@@ -95,12 +97,47 @@ impl Removable {
         })
     }
 
+    /// Format itself as a valid `gio mount $device` argument.
+    fn format_for_gio(&self) -> String {
+        format!("mtp://{name}", name = self.device_name)
+    }
+
+    pub fn mount_simple(&mut self) -> Result<bool> {
+        self.mount("", &mut PasswordHolder::default())
+    }
+
+    pub fn umount_simple(&mut self) -> Result<bool> {
+        self.umount("", &mut PasswordHolder::default())
+    }
+}
+
+impl MountHelper for Removable {
+    /// Parameters used to `sudo mkdir mountpoint`
+    fn format_mkdir_parameters(&self, _username: &str) -> [String; 3] {
+        unreachable!("no need for mkdir when mounting an MTP device")
+    }
+
+    /// Parameters used to mount the device
+    fn format_mount_parameters(&mut self, _username: &str) -> Vec<String> {
+        unreachable!("no need for mount parameters when mounting an MTP device")
+    }
+
+    /// Parameters used to umount the device
+    fn format_umount_parameters(&self, _username: &str) -> Vec<String> {
+        unreachable!("no need for umount parameters when mounting an MTP device")
+    }
+
+    /// True if the device is mounted
+    fn is_mounted(&self) -> bool {
+        self.is_mounted
+    }
+
     /// Mount a non mounted removable device.
     /// `Err` if the device is already mounted.
     /// Runs a `gio mount $device_name` command and check
     /// the result.
     /// The `is_mounted` flag is updated accordingly to the result.
-    pub fn mount(&mut self) -> Result<()> {
+    fn mount(&mut self, _: &str, _: &mut PasswordHolder) -> Result<bool> {
         if self.is_mounted {
             return Err(anyhow!("Already mounted {name}", name = self.device_name));
         }
@@ -116,7 +153,7 @@ impl Removable {
             "Mounted {device}. Success ? {success}",
             device=self.device_name, success=self.is_mounted
         );
-        Ok(())
+        Ok(self.is_mounted)
     }
 
     /// Unount a mounted removable device.
@@ -124,7 +161,7 @@ impl Removable {
     /// Runs a `gio mount $device_name` command and check
     /// the result.
     /// The `is_mounted` flag is updated accordingly to the result.
-    pub fn umount(&mut self) -> Result<()> {
+    fn umount(&mut self, _: &str, _: &mut PasswordHolder) -> Result<bool> {
         if !self.is_mounted {
             return Err(anyhow!("Not mounted {name}", name = self.device_name));
         }
@@ -141,12 +178,16 @@ impl Removable {
             device=self.device_name,
             success=!self.is_mounted
         );
-        Ok(())
+        Ok(!self.is_mounted)
     }
 
-    /// Format itself as a valid `gio mount $device` argument.
-    fn format_for_gio(&self) -> String {
-        format!("mtp://{name}", name = self.device_name)
+    /// String representation of the device
+    fn as_string(&self) -> Result<String> {
+        Ok(self.device_name.to_owned())
+    }
+
+    fn device_name(&self) -> Result<String> {
+        self.as_string()
     }
 }
 
diff --git a/src/status.rs b/src/status.rs
index ea69baa..e700d2e 100644
--- a/src/status.rs
+++ b/src/status.rs
@@ -829,10 +829,10 @@ impl Status {
         let Some(device) = devices.selected_mut() else {
             return Ok(());
         };
-        if device.is_mounted {
+        if device.is_mounted() {
             return Ok(());
         }
-        device.mount()?;
+        device.mount_simple()?;
         Ok(())
     }
 
@@ -843,10 +843,10 @@ impl Status {
         let Some(device) = devices.selected_mut() else {
             return Ok(());
         };
-        if !device.is_mounted {
+        if !device.is_mounted() {
             return Ok(());
         }
-        device.umount()?;
+        device.umount_simple()?;
         Ok(())
     }
 
@@ -857,7 +857,7 @@ impl Status {
         let Some(device) = devices.selected() else {
             return Ok(());
         };
-        if !device.is_mounted {
+        if !device.is_mounted() {
             return Ok(());
         }
         let path = std::path::PathBuf::from(&device.path);
diff --git a/src/term_manager.rs b/src/term_manager.rs
index c05b0d3..178cfc0 100644
--- a/src/term_manager.rs
+++ b/src/term_manager.rs
@@ -864,45 +864,51 @@ impl<'a> WinSecondary<'a> {
     }
 
     fn draw_encrypted_drive(&self, canvas: &mut dyn Canvas) -> Result<()> {
-        canvas.print_with_attr(2, 3, ENCRYPTED_DEVICE_BINDS, Self::ATTR_YELLOW)?;
-        for (i, device) in self.status.encrypted_devices.content.iter().enumerate() {
-            let row = calc_line_row(i, &self.tab.window) + 2;
-            let mut not_mounted_attr = Attr::default();
-            let mut mounted_attr = Attr::from(Color::BLUE);
-            if i == self.status.encrypted_devices.index() {
-                not_mounted_attr.effect |= Effect::REVERSE;
-                mounted_attr.effect |= Effect::REVERSE;
-            }
-            if self.status.encrypted_devices.content[i].is_mounted() {
-                canvas.print_with_attr(row, 3, &device.as_string()?, mounted_attr)?;
-            } else {
-                canvas.print_with_attr(row, 3, &device.as_string()?, not_mounted_attr)?;
-            }
+        self.draw_mountable_devices(&self.status.encrypted_devices, canvas)
+    }
+
+    fn draw_removable(&self, canvas: &mut dyn Canvas) -> Result<()> {
+        if let Some(removables) = &self.status.removable_devices {
+            self.draw_mountable_devices(removables, canvas)?;
         }
         Ok(())
     }
 
-    fn draw_removable(&self, canvas: &mut dyn Canvas) -> Result<()> {
+    fn draw_mountable_devices<T>(
+        &self,
+        selectable: &impl SelectableContent<T>,
+        canvas: &mut dyn Canvas,
+    ) -> Result<()>
+    where
+        T: MountHelper,
+    {
         canvas.print_with_attr(2, 3, ENCRYPTED_DEVICE_BINDS, Self::ATTR_YELLOW)?;
-        if let Some(removable) = &self.status.removable_devices {
-            for (i, device) in removable.content.iter().enumerate() {
-                let row = calc_line_row(i, &self.tab.window) + 2;
-                let mut not_mounted_attr = Attr::default();
-                let mut mounted_attr = Attr::from(Color::BLUE);
-                if i == removable.index {
-                    not_mounted_attr.effect |= Effect::REVERSE;
-                    mounted_attr.effect |= Effect::REVERSE;
-                }
-                if device.is_mounted {
-                    canvas.print_with_attr(row, 3, &device.device_name, mounted_attr)?;
-                } else {
-                    canvas.print_with_attr(row, 3, &device.device_name, not_mounted_attr)?;
-                }
-            }
+        for (i, device) in selectable.content().iter().enumerate() {
+            self.draw_mountable_device(selectable, i, device, canvas)?
         }
         Ok(())
     }
 
+    fn draw_mountable_device<T>(
+        &self,
+        selectable: &impl SelectableContent<T>,
+        index: usize,
+        device: &T,
+        canvas: &mut dyn Canvas,
+    ) -> Result<()>
+    where
+        T: MountHelper,
+    {
+        let row = calc_line_row(index, &self.tab.window) + 2;
+        let attr = if device.is_mounted() {
+            selectable.attr(index, &Attr::from(Color::BLUE))
+        } else {
+            selectable.attr(index, &Attr::default())
+        };
+        canvas.print_with_attr(row, 3, &device.device_name()?, attr)?;
+        Ok(())
+    }
+
     /// Display a list of edited (deleted, copied, moved, trashed) files for confirmation
     fn draw_confirm(
         &self,