Merge pull request #47 from djmitche/issue43

Update handling of working set and show id's more widely

cli/src/cmd/info.rs

@@ -30,11 +30,16 @@ define_subcommand! {
 
 subcommand_invocation! {
     fn run(&self, command: &CommandInvocation) -> Fallible<()> {
-        let task = shared::get_task(&mut command.get_replica(), &self.task)?;
+        let mut replica = command.get_replica();
+        let task = shared::get_task(&mut replica, &self.task)?;
+        let uuid = task.get_uuid();
 
         let mut t = Table::new();
         t.set_format(table::format());
-        t.add_row(row![b->"Uuid", task.get_uuid()]);
+        t.add_row(row![b->"Uuid", uuid]);
+        if let Some(i) = replica.get_working_set_index(uuid)? {
+            t.add_row(row![b->"Id", i]);
+        }
         t.add_row(row![b->"Description", task.get_description()]);
         t.add_row(row![b->"Status", task.get_status()]);
         t.printstd();

cli/src/cmd/list.rs

@@ -23,11 +23,16 @@ define_subcommand! {
 
 subcommand_invocation! {
     fn run(&self, command: &CommandInvocation) -> Fallible<()> {
+        let mut replica = command.get_replica();
         let mut t = Table::new();
         t.set_format(table::format());
-        t.set_titles(row![b->"uuid", b->"description"]);
+        t.set_titles(row![b->"id", b->"description"]);
-        for (uuid, task) in command.get_replica().all_tasks().unwrap() {
+        for (uuid, task) in replica.all_tasks().unwrap() {
-            t.add_row(row![uuid, task.get_description()]);
+            let mut id = uuid.to_string();
+            if let Some(i) = replica.get_working_set_index(&uuid)? {
+                id = i.to_string();
+            }
+            t.add_row(row![id, task.get_description()]);
         }
         t.printstd();
         Ok(())

cli/src/cmd/shared.rs

@@ -12,13 +12,10 @@ pub(super) fn get_task<S: AsRef<str>>(replica: &mut Replica, task_arg: S) -> Fal
     let task_arg = task_arg.as_ref();
 
     // first try treating task as a working-set reference
-    match task_arg.parse::<u64>() {
+    match task_arg.parse::<usize>() {
         Ok(i) => {
-            let mut working_set = replica.working_set().unwrap();
+            if let Some(task) = replica.get_working_set_task(i)? {
-            if i > 0 && i < working_set.len() as u64 {
+                return Ok(task);
-                if let Some(task) = working_set[i as usize].take() {
-                    return Ok(task);
-                }
             }
         }
         Err(_) => {}

taskchampion/src/replica.rs

@@ -48,7 +48,7 @@ impl Replica {
     }
 
     /// Add the given uuid to the working set, returning its index.
-    pub(crate) fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<u64> {
+    pub(crate) fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize> {
         self.taskdb.add_to_working_set(uuid)
     }
 
@@ -92,7 +92,7 @@ impl Replica {
     }
 
     /// Get an existing task by its working set index
-    pub fn get_working_set_task(&mut self, i: u64) -> Fallible<Option<Task>> {
+    pub fn get_working_set_task(&mut self, i: usize) -> Fallible<Option<Task>> {
         let working_set = self.taskdb.working_set()?;
         if (i as usize) < working_set.len() {
             if let Some(uuid) = working_set[i as usize] {
@@ -105,6 +105,19 @@ impl Replica {
         return Ok(None);
     }
 
+    /// Get the working set index for the given task uuid
+    pub fn get_working_set_index(&mut self, uuid: &Uuid) -> Fallible<Option<usize>> {
+        let working_set = self.taskdb.working_set()?;
+        for (i, u) in working_set.iter().enumerate() {
+            if let Some(ref u) = u {
+                if u == uuid {
+                    return Ok(Some(i));
+                }
+            }
+        }
+        Ok(None)
+    }
+
     /// Create a new task.  The task must not already exist.
     pub fn new_task(&mut self, uuid: Uuid, status: Status, description: String) -> Fallible<Task> {
         // check that it doesn't exist; this is a convenience check, as the task
@@ -223,6 +236,10 @@ mod tests {
         let t = rep.get_task(&uuid).unwrap().unwrap();
         assert_eq!(t.get_status(), Status::Deleted);
         assert_eq!(t.get_description(), "gone");
+
+        rep.gc().unwrap();
+
+        assert!(rep.get_working_set_index(t.get_uuid()).unwrap().is_none());
     }
 
     #[test]
@@ -241,6 +258,8 @@ mod tests {
         assert_eq!(ws.len(), 2);
         assert!(ws[0].is_none());
         assert_eq!(ws[1].as_ref().unwrap().get_uuid(), &uuid);
+
+        assert_eq!(rep.get_working_set_index(t.get_uuid()).unwrap().unwrap(), 1);
     }
 
     #[test]

taskchampion/src/taskdb.rs

@@ -153,13 +153,13 @@ impl TaskDB {
 
     /// Add the given uuid to the working set and return its index; if it is already in the working
     /// set, its index is returned.  This does *not* renumber any existing tasks.
-    pub fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<u64> {
+    pub fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize> {
         let mut txn = self.storage.txn()?;
         // search for an existing entry for this task..
         for (i, elt) in txn.get_working_set()?.iter().enumerate() {
             if *elt == Some(*uuid) {
                 // (note that this drops the transaction with no changes made)
-                return Ok(i as u64);
+                return Ok(i);
             }
         }
         // and if not found, add one

taskchampion/src/taskstorage/inmemory.rs

@@ -108,14 +108,13 @@ impl<'t> TaskStorageTxn for Txn<'t> {
         Ok(self.data_ref().working_set.clone())
     }
 
-    fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<u64> {
+    fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize> {
         let working_set = &mut self.mut_data_ref().working_set;
         working_set.push(Some(uuid.clone()));
-        Ok(working_set.len() as u64)
+        Ok(working_set.len())
     }
 
-    fn remove_from_working_set(&mut self, index: u64) -> Fallible<()> {
+    fn remove_from_working_set(&mut self, index: usize) -> Fallible<()> {
-        let index = index as usize;
         let working_set = &mut self.mut_data_ref().working_set;
         if index >= working_set.len() || working_set[index].is_none() {
             return Err(format_err!("No task found with index {}", index));

taskchampion/src/taskstorage/kv.rs

@@ -302,7 +302,7 @@ impl<'t> TaskStorageTxn for Txn<'t> {
         Ok(res)
     }
 
-    fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<u64> {
+    fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize> {
         let working_set_bucket = self.working_set_bucket();
         let numbers_bucket = self.numbers_bucket();
         let kvtxn = self.kvtxn();
@@ -323,10 +323,11 @@ impl<'t> TaskStorageTxn for Txn<'t> {
             NEXT_WORKING_SET_INDEX.into(),
             Msgpack::to_value_buf(next_index + 1)?,
         )?;
-        Ok(next_index)
+        Ok(next_index as usize)
     }
 
-    fn remove_from_working_set(&mut self, index: u64) -> Fallible<()> {
+    fn remove_from_working_set(&mut self, index: usize) -> Fallible<()> {
+        let index = index as u64;
         let working_set_bucket = self.working_set_bucket();
         let numbers_bucket = self.numbers_bucket();
         let kvtxn = self.kvtxn();

taskchampion/src/taskstorage/mod.rs

@@ -80,10 +80,10 @@ pub trait TaskStorageTxn {
 
     /// Add a task to the working set and return its (one-based) index.  This index will be one greater
     /// than the highest used index.
-    fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<u64>;
+    fn add_to_working_set(&mut self, uuid: &Uuid) -> Fallible<usize>;
 
     /// Remove a task from the working set.  Other tasks' indexes are not affected.
-    fn remove_from_working_set(&mut self, index: u64) -> Fallible<()>;
+    fn remove_from_working_set(&mut self, index: usize) -> Fallible<()>;
 
     /// Clear all tasks from the working set in preparation for a garbage-collection operation.
     fn clear_working_set(&mut self) -> Fallible<()>;