Merge branch 'libimaghabit-dont-copy-comment-to-instance' into master

bin/domain/imag-habit/src/main.rs

@@ -480,12 +480,12 @@ fn show(rt: &Runtime) {
         .map(String::from)
         .unwrap(); // safe by clap
 
-    fn instance_lister_fn(i: &FileLockEntry) -> Vec<String> {
+    fn instance_lister_fn(rt: &Runtime, i: &FileLockEntry) -> Vec<String> {
         use libimagutil::date::date_to_string;
         use libimaghabit::instance::HabitInstance;
 
         let date = date_to_string(&i.get_date().map_err_trace_exit_unwrap());
-        let comm = i.get_comment().map_err_trace_exit_unwrap();
+        let comm = i.get_comment(rt.store()).map_err_trace_exit_unwrap();
 
         vec![date, comm]
     }
@@ -523,7 +523,7 @@ fn show(rt: &Runtime) {
                 .unwrap_or_exit();
 
             let mut empty = true;
-            let _ = habit
+            let iter = habit
                 .linked_instances()
                 .map_err_trace_exit_unwrap()
                 .trace_unwrap_exit()
@@ -531,10 +531,17 @@ fn show(rt: &Runtime) {
                     debug!("Getting: {:?}", instance_id);
                     rt.store().get(instance_id).map_err_trace_exit_unwrap()
                 })
-                .enumerate()
+                .enumerate();
-                .for_each(|(i, e)| {
+
+            // We need to drop here because we iterate over instances and in the
+            // instance_lister_fn() we call instance.get_comment(), which internally tries to
+            // Store::get() the template object.
+            // This would fail because the template is already borrowed.
+            drop(habit);
+
+            iter.for_each(|(i, e)| {
                     let mut v = vec![format!("{}", i)];
-                    let mut instances = instance_lister_fn(&e);
+                    let mut instances = instance_lister_fn(&rt, &e);
 
                     {
                         let _ = rt.report_touched(e.get_location()).unwrap_or_exit();

lib/domain/libimaghabit/src/habit.rs

@@ -99,13 +99,12 @@ impl HabitTemplate for Entry {
 
     fn create_instance_with_date<'a>(&mut self, store: &'a Store, date: &NaiveDate) -> Result<FileLockEntry<'a>> {
         let name    = self.habit_name()?;
-        let comment = self.habit_comment()?;
         let date    = date_to_string(date);
         let id      = instance_id_for_name_and_datestr(&name, &date)?;
 
         store.create(id)
             .map_err(From::from)
-            .and_then(|entry| postprocess_instance(entry, name, date, comment, self))
+            .and_then(|entry| postprocess_instance(entry, name, date, self))
     }
 
     fn create_instance_today<'a>(&mut self, store: &'a Store) -> Result<FileLockEntry<'a>> {
@@ -114,13 +113,12 @@ impl HabitTemplate for Entry {
 
     fn retrieve_instance_with_date<'a>(&mut self, store: &'a Store, date: &NaiveDate) -> Result<FileLockEntry<'a>> {
         let name    = self.habit_name()?;
-        let comment = self.habit_comment()?;
         let date    = date_to_string(date);
         let id      = instance_id_for_name_and_datestr(&name, &date)?;
 
         store.retrieve(id)
             .map_err(From::from)
-            .and_then(|entry| postprocess_instance(entry, name, date, comment, self))
+            .and_then(|entry| postprocess_instance(entry, name, date, self))
     }
 
     fn retrieve_instance_today<'a>(&mut self, store: &'a Store) -> Result<FileLockEntry<'a>> {
@@ -398,7 +396,6 @@ pub mod builder {
 fn postprocess_instance<'a>(mut entry: FileLockEntry<'a>,
                             name: String,
                             date: String,
-                            comment: String,
                             template: &mut Entry)
     -> Result<FileLockEntry<'a>>
 {
@@ -407,7 +404,6 @@ fn postprocess_instance<'a>(mut entry: FileLockEntry<'a>,
         let hdr = entry.get_header_mut();
         let _   = hdr.insert("habit.instance.name",    Value::String(name))?;
         let _   = hdr.insert("habit.instance.date",    Value::String(date))?;
-        let _   = hdr.insert("habit.instance.comment", Value::String(comment))?;
     }
 
     entry.add_link(template)?;

lib/domain/libimaghabit/src/instance.rs

@@ -17,16 +17,21 @@
 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
 //
 
+use std::ops::Deref;
+
 use chrono::NaiveDate;
 use toml::Value;
 use toml_query::set::TomlValueSetExt;
 use failure::Fallible as Result;
 
 use crate::util::*;
+use crate::habit::HabitTemplate;
 
 use libimagstore::store::Entry;
+use libimagstore::store::Store;
 use libimagentryutil::isa::Is;
 use libimagentryutil::isa::IsKindHeaderPathProvider;
+use libimagentrylink::linkable::Linkable;
 
 /// An instance of a habit is created for each time a habit is done.
 ///
@@ -41,8 +46,7 @@ pub trait HabitInstance {
 
     fn get_date(&self) -> Result<NaiveDate>;
     fn set_date(&mut self, n: &NaiveDate) -> Result<()>;
-    fn get_comment(&self) -> Result<String>;
+    fn get_comment(&self, store: &Store) -> Result<String>;
-    fn set_comment(&mut self, c: String) -> Result<()>;
     fn get_template_name(&self) -> Result<String>;
 }
 
@@ -68,16 +72,37 @@ impl HabitInstance for Entry {
             .map(|_| ())
     }
 
-    fn get_comment(&self) -> Result<String> {
+    /// Iterates all internal links, finds the template for this instance and gets the comment from
-        get_string_header_from_entry(self, "habit.instance.comment")
+    /// it
-    }
+    ///
-
+    ///
-    fn set_comment(&mut self, c: String) -> Result<()> {
+    /// # Warning
-        // Using `set` here because when creating the entry, these headers should be made present.
+    ///
-        self.get_header_mut()
+    /// Internally tries to `Store::get()` the template entry. If this entry is borrowed outside of
-            .set("habit.instance.comment", Value::String(c))
+    /// this function, this fails.
-            .map_err(From::from)
+    ///
-            .map(|_| ())
+    /// If multiple templates are linked to this entry, this returns the comment of the first
+    ///
+    ///
+    /// # Return
+    ///
+    /// Returns the Comment string from the first template that is linked to this instance.
+    /// If this is not an instance, this might misbehave.
+    /// If there is no template linked, this returns an error.
+    ///
+    fn get_comment(&self, store: &Store) -> Result<String> {
+        let templ_name = self.get_template_name()?;
+        for link in self.links()? {
+            let template = store.get(link.get_store_id().clone())?.ok_or_else(|| {
+                format_err!("Entry {} is linked to {}, but that entry does not exist",
+                            self.get_location(),
+                            link.get_store_id())
+            })?;
+            if HabitTemplate::is_habit_template(template.deref())? && template.habit_name()? == templ_name {
+                return template.habit_comment()
+            }
+        }
+        Err(format_err!("Cannot find template entry for {}", self.get_location()))
     }
 
     fn get_template_name(&self) -> Result<String> {