From ae41a775211ff2da25627bee9aec5f5ab860f6eb Mon Sep 17 00:00:00 2001
From: Christopher Davis <christopherdavis@gnome.org>
Date: Thu, 2 Mar 2023 21:30:57 -0500
Subject: [PATCH] properties_view: Move expressions to UI files

Currently Loupe leaks expressions created in-code. This
is because expressions need to be unwatched in order to be
dropped. Since we'd need to hold a reference to the watch
ourselves and unwatch them in dispose, our code would
become more cumbersome if we were to try to handle this properly.

This commit moves the expressions on LpPropertiesView to the
UI file, as GTK handles cleaning up the expressions it creates,
without any extra consideration on the app side.
---
 data/gtk/properties_view.ui    | 10 +++++++++
 src/widgets/properties_view.rs | 40 ++++++++--------------------------
 2 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/data/gtk/properties_view.ui b/data/gtk/properties_view.ui
index d97e0fd..ddfca20 100644
--- a/data/gtk/properties_view.ui
+++ b/data/gtk/properties_view.ui
@@ -2,6 +2,16 @@
 <interface>
   <template class="LpPropertiesView" parent="AdwBin">
     <property name="width-request">255</property>
+    <binding name="metadata">
+      <lookup type="LpImage" name="metadata">
+        <lookup type="LpPropertiesView" name="image"/>
+      </lookup>
+    </binding>
+    <binding name="file">
+      <lookup type="LpImage" name="file">
+        <lookup type="LpPropertiesView" name="image"/>
+      </lookup>
+    </binding>
     <property name="child">
       <object class="GtkScrolledWindow">
         <property name="child">
diff --git a/src/widgets/properties_view.rs b/src/widgets/properties_view.rs
index 91eccef..20a436b 100644
--- a/src/widgets/properties_view.rs
+++ b/src/widgets/properties_view.rs
@@ -102,9 +102,7 @@ mod imp {
                     glib::ParamSpecObject::builder::<LpImage>("image")
                         .explicit_notify()
                         .build(),
-                    glib::ParamSpecObject::builder::<LpImageMetadata>("metadata")
-                        .read_only()
-                        .build(),
+                    glib::ParamSpecObject::builder::<LpImageMetadata>("metadata").build(),
                 ]
             });
 
@@ -131,6 +129,8 @@ mod imp {
 
             match prop_name {
                 "image" => obj.set_image(value.get().ok().as_ref()),
+                "file" => obj.set_file(value.get().ok().as_ref()),
+                "metadata" => obj.set_metadata(value.get().unwrap()),
                 _ => unimplemented!(),
             }
         }
@@ -140,34 +140,6 @@ mod imp {
 
             self.parent_constructed();
             obj.action_set_enabled("properties.open-folder", false);
-
-            // watch changes to image metadata
-            obj.property_expression("image")
-                .chain_property::<LpImage>("metadata")
-                .watch(
-                    glib::Object::NONE,
-                    glib::clone!(@weak obj => move || {
-                        let metadata = if let Some(image) = obj.image() {
-                            image.metadata()
-                        } else {
-                            LpImageMetadata::default()
-                        };
-
-                        // set metadata since image might have changed
-                        obj.imp().metadata.replace(metadata);
-                        obj.notify("metadata");
-                    }),
-                );
-
-            // watch changes to image metadata
-            obj.property_expression("image")
-                .chain_property::<LpImage>("path")
-                .watch(
-                    glib::Object::NONE,
-                    glib::clone!(@weak obj => move || {
-                        obj.set_file(obj.image().and_then(|x| x.file()).as_ref());
-                    }),
-                );
         }
     }
 
@@ -276,6 +248,12 @@ impl LpPropertiesView {
         });
     }
 
+    fn set_metadata(&self, metadata: LpImageMetadata) {
+        let imp = self.imp();
+        imp.metadata.replace(metadata);
+        self.notify("metadata");
+    }
+
     // This is where we handle the results of the future.
     // This could technically be part of the future, but we keep
     // them separate for a bit of cleanliness.