From 68221bacba3078c9bb4ed84117464c3cb8441126 Mon Sep 17 00:00:00 2001
From: Sophie Herold <sophie@hemio.de>
Date: Sat, 26 Nov 2022 00:02:56 +0100
Subject: [PATCH] file_model: Load opened image first add dir later

- Fixes image cannot be opened when directory unreadable
- Avoids bugs with wrong images being selected for an instant.
  Mostly important for adjusting the window size to image.
- Basis for prioritizing loading the important image first
---
 src/file_model.rs         | 33 ++++++++++++++++-----
 src/widgets/image_view.rs | 60 ++++++++++++++++++++++-----------------
 2 files changed, 60 insertions(+), 33 deletions(-)

diff --git a/src/file_model.rs b/src/file_model.rs
index 84e1f0d..394d735 100644
--- a/src/file_model.rs
+++ b/src/file_model.rs
@@ -70,12 +70,30 @@ glib::wrapper! {
 }
 
 impl LpFileModel {
-    pub fn from_directory(directory: &gio::File) -> anyhow::Result<Self> {
+    pub fn from_file(file: &gio::File) -> anyhow::Result<Self> {
         let model = glib::Object::new::<Self>(&[]);
+        let directory = file.parent().context("File has not parent")?;
+        let file =
+            directory.resolve_relative_path(file.basename().context("File has no basename")?);
+
+        {
+            let mut vec = model.imp().inner.borrow_mut();
+            vec.push(file);
+        }
+
+        model.imp().directory.set(directory.clone()).unwrap();
+
+        Ok(model)
+    }
+
+    pub fn load_directory(&self) -> anyhow::Result<()> {
+        let directory = self.imp().directory.get().unwrap();
 
         {
             // Here we use a nested scope so that the mutable borrow only lasts as long as we need it
-            let mut vec = model.imp().inner.borrow_mut();
+            let mut vec = self.imp().inner.borrow_mut();
+            let original_vec: Vec<Option<std::path::PathBuf>> =
+                vec.iter().map(|x| x.path()).collect();
 
             let enumerator = directory
                 .enumerate_children(
@@ -95,8 +113,11 @@ impl LpFileModel {
                     if let Some(content_type) = info.content_type().map(|t| t.to_string()) {
                         if content_type.starts_with("image/") {
                             let name = info.name();
-                            log::debug!("{:?} is an image, adding to the list", name);
-                            vec.push(directory.resolve_relative_path(&name));
+                            let relative_path = directory.resolve_relative_path(&name);
+                            if !original_vec.contains(&relative_path.path()) {
+                                log::debug!("{:?} is an image, adding to the list", name);
+                                vec.push(relative_path);
+                            }
                         }
                     }
                 }
@@ -104,11 +125,9 @@ impl LpFileModel {
 
             // Then sort by name.
             vec.sort_by(util::compare_by_name);
-
-            model.imp().directory.set(directory.clone()).unwrap();
         }
 
-        Ok(model)
+        Ok(())
     }
 
     pub fn directory(&self) -> Option<gio::File> {
diff --git a/src/widgets/image_view.rs b/src/widgets/image_view.rs
index af36079..dd39dc3 100644
--- a/src/widgets/image_view.rs
+++ b/src/widgets/image_view.rs
@@ -182,38 +182,46 @@ impl LpImageView {
         let imp = self.imp();
         let carousel = &imp.carousel;
 
-        {
+        let model = {
             // Here we use a nested scope so that the mutable borrow only lasts as long as we need it
-            let mut model = imp.model.borrow_mut();
+            let mut model_store = imp.model.borrow_mut();
 
-            if let Some(ref parent) = file.parent() {
-                if let Some(ref m) = *model {
-                    if m.directory().map_or(false, |f| !f.equal(parent)) {
-                        // Clear the carousel before creating the new model
-                        self.clear_carousel(false);
-                        *model = Some(LpFileModel::from_directory(parent)?);
-                        log::debug!("new model created");
-                    } else {
-                        log::debug!("Re-using old model and navigating to the current file");
-                        self.navigate_to_file(m, file);
-                        return Ok(());
-                    }
-                } else {
-                    *model = Some(LpFileModel::from_directory(parent)?);
+            if let Some(ref m) = *model_store {
+                let is_same_directory = file
+                    .parent()
+                    .map_or(false, |p| m.directory().map_or(false, |f| !f.equal(&p)));
+
+                if is_same_directory {
+                    // Clear the carousel before creating the new model
+                    self.clear_carousel(false);
+                    let model = LpFileModel::from_file(file)?;
+                    *model_store = Some(model.clone());
                     log::debug!("new model created");
+                    model
+                } else {
+                    log::debug!("Re-using old model and navigating to the current file");
+                    self.navigate_to_file(m, file);
+                    return Ok(());
                 }
+            } else {
+                let model = LpFileModel::from_file(file)?;
+                *model_store = Some(model.clone());
+                log::debug!("new model created");
+                model
             }
-        }
+        };
 
-        if let Some(model) = imp.model.borrow().as_ref() {
-            let index = model
-                .index_of(file)
-                .context(i18n("File not found in model. (TODO: bad message)"))?;
-            log::debug!("Currently at file {index} in the directory");
-            carousel.append(&LpImagePage::from_file(file));
-            self.fill_carousel(model, index);
-            self.update_action_state(model, index);
-        }
+        carousel.append(&LpImagePage::from_file(file));
+        self.update_action_state(&model, 0);
+
+        model.load_directory()?;
+        let index = model
+            .index_of(file)
+            .context(i18n("File not found in model. (TODO: bad message)"))?;
+        log::debug!("Currently at file {index} in the directory");
+
+        self.fill_carousel(&model, index);
+        self.update_action_state(&model, index);
 
         Ok(())
     }