Switch to taskwarrior v3.X backend #546
Conversation
| .ok_or_else(|| anyhow!("Unable to get task files max time")) | ||
| fn get_task_database_mtime(&self) -> Result<SystemTime> { | ||
| let data_dir = shellexpand::tilde(&self.config.data_location); | ||
| let database_path = Path::new(data_dir.as_ref()).join("taskchampion.sqlite3"); |
There was a problem hiding this comment.
I haven't had the chance to look into taskwarrior v3 yet, but isn't taskchampion an entirely different tool?
There was a problem hiding this comment.
The project is a bit hidden in the subfolder of the taskwarrior repository (taskchampion README).
As far as I understood it used to be it's own separate tool up until 2 years ago.
Taskchampion is now the new storage backend for taskwarrior, as far as I understand it.
To quote:
TaskChampion implements the task storage and synchronization behind Taskwarrior. It includes an implementation with Rust and C APIs, allowing any application to maintain and manipulate its own replica. It also includes a specification for tasks and how they are synchronized, inviting alternative implementations of replicas or task servers.
See the documentation for more!
NOTE: Taskwarrior is currently in the midst of a change to use TaskChampion as its storage. Until that is complete, the information here may be out-of-date.
The taskchampion.sqlite3 gets created during the v2->v3 import phase, deprecating the .data files.
(The docs seem still a bit unstable, but I'm planning to read a bit more into taskchampions and libshared over the next weeks. Maybe I'll discover some more potential performance boosts?)
There was a problem hiding this comment.
Update: I ran into a problem with the tests, where a specific one fails after the taskwarrior update. I haven't fully understood the reason why yet, as I haven't looked through the testdata(and therefore don't quite get why test_draw_task_report_with_extended_modify_command expects there to be 26 tasks).
If I could ask you a few questions about the codebase after you get back from your vaccation(I don't know how to reach you outside of Github and asking every question on github gets a bit awkward), that would be great.
What will also (soft) block the merge for now is the changes which read-only commands now modify the database mtime(due to the new urgency setting getting updated).
Softblocked because this sends two active tui instances in a loop of running update, therefore modifying the database which then triggers the update of the other instance.
(Is more than 1 instance active a supported use case, or did that run into bugs even before the upgrade?)
I really have to read through the taskchampion docu, the replika structure should be what is in-built to avoid such scenarios.
There was a problem hiding this comment.
The reason for the failing tests was most likely that I forgot to bump the taskwarrior version for CI/CD.
(As they ran error free locally)
Should be fixed now, it will require a manual approval from you to run though.
There was a problem hiding this comment.
Thanks so much for making the PR and for working on this! I'm back from my vacation, happy to answer any questions.
In the meantime, I'll look into this more as well. I will update my local install to the latest version of taskwarrior to test this PR.
There was a problem hiding this comment.
Thanks so much for making the PR and for working on this! I'm back from my vacation, happy to answer any questions.
In the meantime, I'll look into this more as well. I will update my local install to the latest version of taskwarrior to test this PR.
Glad that I can be of help!
What's the best way to reach you, this PR is getting a bit hard to read.
I could offer Matrix/Telegram/Signal/E-Mail(including GPG) etc.
We could alternatively use the Github Discussions feature.
(A first question would be the reasoning behind the current release options. Are incremental, debug and disabled lto a leftover from debugging?)
There was a problem hiding this comment.
Email is probably easiest! GitHub Discussions works too. I'm on the Ratatui discord if you want to ping me there that works too. I don't actively check Matrix but could sign up to if you want to chat there.
There was a problem hiding this comment.
I just sent you a DM in Discord(this message is meant as a second factor that I sent it. I'd like to avoid linking my Discord to my Github directly)
| @@ -3763,13 +3767,22 @@ pub fn remove_tag(task: &mut Task, tag: &str) { | |||
| } | |||
|
|
|||
| #[cfg(test)] | |||
There was a problem hiding this comment.
I took the liberty of applying some outstanding clippy suggestions.
Most of these should be negligible, correct me if I'm wrong.
Two are imo controversial but justifiable, number one is this lint.
Context:
let shell = message.as_str().replace("'", "'");
triggers the clippy warning for a single char pattern, it's suggestion ''' is imo more confusing than helpful.
| mod tests { | ||
| use std::{ffi::OsStr, fmt::Write, fs::File, io, path::Path}; | ||
| use std::{ffi::OsStr, fmt::Write, fs::File, io, path::{Path, PathBuf}}; | ||
|
|
||
| use ratatui::{backend::TestBackend, buffer::Buffer}; | ||
|
|
||
| use super::*; |
There was a problem hiding this comment.
The second, potentially more controversial one(as I don't know if there was specific reasoning for compile time embedding).
Mostly a minor improvement, intended so that the different TASKDATA can be specified for the tests without a recompile.
It has the additional free benefit that it avoids 4 compiler warnings if the developers chosen IDE needs additional work to start rust-analzyer in the direnv context(e.g. VSCode requires the direnv extension. It is not required for the inbuilt terminal, so VSCode seems to do some shell magic under the hood).
Addresses #545.
See my comment on the issue for additional details.
I chose to drop Support for Taskwarrior v2.X.
This should avoid, while not technically necessary, ugly version specific fixes in the code, if significant differences between v2.X and v3.X surface.