Add a (mostly) full Task FFI from TaskChampion#4117
Conversation
carmiac
changed the title
|
I apologize for my tardiness in looking at this! |
djmitche
left a comment
djmitche
left a comment
There was a problem hiding this comment.
Again, I'm sorry for taking well over a month to review this. It looks great, with just a few very minor updates below. Otherwise I'm happy with this, but I would like to understand what the next step is before doing so. What's the benefit of this approach over the lower-level TaskData type?
|
|
||
| /// Add a tag to the task. The tag is parsed from a string, so can fail. | ||
| fn add_tag(self: &mut Task, tag: &CxxString, ops: &mut Vec<Operation>) -> Result<()>; | ||
| /// Remove a tag from the task. The tag is parsed from a, so can fail. |
There was a problem hiding this comment.
| /// Remove a tag from the task. The tag is parsed from a, so can fail. | |
| /// Remove a tag from the task. The tag is parsed from a string, so can fail. |
| /// This contains `OptionTaskData` to allow C++ to `take` values out of the vector and use | ||
| /// them as `rust::Box<TaskData>`. Cxx does not support `Vec<Box<_>>`. Cxx also does not |
There was a problem hiding this comment.
| /// This contains `OptionTaskData` to allow C++ to `take` values out of the vector and use | |
| /// them as `rust::Box<TaskData>`. Cxx does not support `Vec<Box<_>>`. Cxx also does not | |
| /// This contains `OptionTask` to allow C++ to `take` values out of the vector and use | |
| /// them as `rust::Box<Task>`. See the comments on `all_task_data`. |
| if let Some(t) = self.0.get_entry() { | ||
| t.timestamp() | ||
| } else { | ||
| 0 | ||
| } |
There was a problem hiding this comment.
A helper function for this -- inverse of optional_timestamp -- might be useful.
| ffi::Status::Completed => tc::Status::Completed, | ||
| ffi::Status::Deleted => tc::Status::Deleted, | ||
| ffi::Status::Recurring => tc::Status::Recurring, | ||
| _ => unreachable!("ffi::Status variants are exhaustive"), |
There was a problem hiding this comment.
This isn't actually unreachable, though -- t.set_status(ffi::Status::Unknown, ops) will reach this point. So I don't think it should panic. It might make sense to treat this as an error, since we can't round-trip the Unknown(value) variant from Rust.
Oh I get it, it's been a rough month for me. This is one part of splitting up #4113 Iterative tasks need to be handled by |
|
Gotcha - thanks! |
2 participants
This adds a mostly complete Task FFI from TaskChampion. It also adds the Replica functions dealing with Tasks.
It does not include:
Task::into_task_dataReplica::dependancy_mapNor does it actually connect any of this to the TaskWarrior logic.