Skip to content

Commit

Permalink
Add positive tag literals as tags to newly added task on empty view
Browse files Browse the repository at this point in the history
When a view (or tab) currently has no tasks on it, adding a new task
cannot use the currently selected task as the "template" to copy tags
from. What currently happens is that the task is simply created without
any tags at all. That may result in a potentially horrible user
experience: if the user has no view configured that displays tasks
without any tags, the task will effectively end up in nirvana. The only
way to fix that is to manually edit the task's configuration file and
add the desired tags.
To fix this problem, this change makes sure to add all tags used as
positive tag literals in the view the task is added to (if the view is
empty). That may result in a few potentially unnecessary tags to be
present (depending on the complexity of the user's view configuration),
but it will result in the newly added task being displayed on the very
view in question, which allows for quick modification of the tags. While
this solution is not perfect (it is still only a heuristic: there is no
"right" way to to about it, aside from perhaps forcing the user to
select the tags to be present as part of task addition), it results in a
strictly better user experience compared to what we currently have.
  • Loading branch information
d-e-s-o committed Apr 26, 2023
1 parent ded2e87 commit 90ce3df
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Unreleased
running, to prevent accidental overwriting of intentional changes
- Properly support scrolling when editing tasks not fitting on the
screen
- Set positive tag literals of tab as tags on a task newly added to an
empty tab
- Removed support for writing output to a file provided as program
argument
- Added support for `--version`/`-V` option to print program version
Expand Down
11 changes: 9 additions & 2 deletions src/ui/task_list_box.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2018-2022 Daniel Mueller (deso@posteo.net)
// Copyright (C) 2018-2023 Daniel Mueller (deso@posteo.net)
// SPDX-License-Identifier: GPL-3.0-or-later

use std::cmp::min;
Expand Down Expand Up @@ -401,7 +401,14 @@ impl Handleable<Event, Message> for TaskListBox {
.collect()
})
} else {
Default::default()
// If there is no selected task to take as a
// "template", fall back to assigning all positive
// literals from the view as tags. The user can always
// deselect them, but having it show up without tags
// (which would be the only other way we can conjure
// up to handle this case), is much worse of a user
// experience.
data.view.positive_tag_iter().cloned().collect()
};

// We want the new task to be displayed after the
Expand Down
29 changes: 28 additions & 1 deletion src/ui/termui.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2017-2022 Daniel Mueller (deso@posteo.net)
// Copyright (C) 2017-2023 Daniel Mueller (deso@posteo.net)
// SPDX-License-Identifier: GPL-3.0-or-later

use std::ffi::OsString;
Expand Down Expand Up @@ -911,6 +911,33 @@ mod tests {
assert_eq!(tags, expected);
}

/// Make sure that adding a task on a currently empty view adds the
/// view's positive tag literals as tags to the task.
#[test]
async fn add_task_on_empty_view() {
let events = vec![
Event::from('l'),
Event::from('l'),
Event::from('a'),
Event::from('f'),
Event::from('o'),
Event::from('o'),
Event::from('\n'),
];

let mut builder = TestUiBuilder::with_default_tasks_and_tags();
// Clear out all tasks.
builder.task_state.tasks = SerTasks::from(Vec::new());

let tasks = builder.build().await.handle(events).await.tasks().await;
assert_eq!(tasks.len(), 1);
assert_eq!(tasks[0].summary(), "foo");

let tags = tasks[0].tags(|iter| iter.map(|x| x.name().to_string()).collect::<Vec<_>>());
let expected = vec!["tag2", "tag3"];
assert_eq!(tags, expected);
}

#[test]
async fn complete_task() {
let events = vec![
Expand Down
70 changes: 69 additions & 1 deletion src/view.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (C) 2017-2022 Daniel Mueller (deso@posteo.net)
// Copyright (C) 2017-2023 Daniel Mueller (deso@posteo.net)
// SPDX-License-Identifier: GPL-3.0-or-later

use std::rc::Rc;
Expand Down Expand Up @@ -299,6 +299,17 @@ impl View {
self.tasks.iter(|iter| f(Filter::new(iter, &self.lits)))
}

/// Retrieve an iterator over all tags of the positive literals in
/// this `View`.
pub fn positive_tag_iter(&self) -> impl Iterator<Item = &Tag> {
self.lits.iter().flat_map(|disjunctions| {
disjunctions.iter().filter_map(|literal| match literal {
TagLit::Pos(tag) => Some(tag),
TagLit::Neg(..) => None,
})
})
}

/// Check whether the view is empty or not.
#[cfg(test)]
pub fn is_empty(&self) -> bool {
Expand Down Expand Up @@ -361,12 +372,69 @@ mod tests {
}


/// Check that we can identify an empty `View`.
#[test]
fn is_empty() {
assert!(make_view(0).is_empty());
assert!(!make_view(1).is_empty());
}

/// Make sure that we can retrieve an iterator over all positive tag
/// literals in a `View`.
#[test]
fn pos_tag_iteration() {
fn pos_tags(view: &View) -> Vec<String> {
view
.positive_tag_iter()
.map(|tag| tag.name().to_string())
.collect::<Vec<_>>()
}

let (templates, tasks) = make_tagged_tasks(20);

let view = ViewBuilder::new(tasks.clone()).build("test");
assert_eq!(pos_tags(&view), Vec::<String>::new());

let view = ViewBuilder::new(tasks.clone())
.and(templates.instantiate_from_name(COMPLETE_TAG))
.build("test");
assert_eq!(pos_tags(&view), vec![COMPLETE_TAG]);

let view = ViewBuilder::new(tasks.clone())
.and(templates.instantiate_from_name("tag1"))
.and_not(templates.instantiate_from_name("tag2"))
.build("test");
assert_eq!(pos_tags(&view), vec!["tag1"]);

let view = ViewBuilder::new(tasks.clone())
.and(templates.instantiate_from_name("tag1"))
.or(templates.instantiate_from_name("tag2"))
.build("test");
assert_eq!(pos_tags(&view), vec!["tag1", "tag2"]);

let view = ViewBuilder::new(tasks.clone())
.and(templates.instantiate_from_name("tag3"))
.or(templates.instantiate_from_name("tag1"))
.and_not(templates.instantiate_from_name("tag2"))
.build("test");
assert_eq!(pos_tags(&view), vec!["tag3", "tag1"]);

let view = ViewBuilder::new(tasks.clone())
.or(templates.instantiate_from_name("tag3"))
.or(templates.instantiate_from_name("tag2"))
.or(templates.instantiate_from_name("tag5"))
.build("test");
assert_eq!(pos_tags(&view), vec!["tag3", "tag2", "tag5"]);

let view = ViewBuilder::new(tasks)
.and(templates.instantiate_from_name("tag3"))
.and(templates.instantiate_from_name("tag2"))
.and(templates.instantiate_from_name("tag5"))
.build("test");
assert_eq!(pos_tags(&view), vec!["tag3", "tag2", "tag5"]);
}

/// Check that we can correctly filter all completed tasks.
#[test]
fn filter_completions() {
let (templates, tasks) = make_tagged_tasks(16);
Expand Down

0 comments on commit 90ce3df

Please sign in to comment.