Skip to content

Commit

Permalink
Issue warnings instead of errors for 'weird' tasks
Browse files Browse the repository at this point in the history
  • Loading branch information
djmitche committed Oct 13, 2024
1 parent 26c383d commit 64f73b4
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 18 deletions.
36 changes: 20 additions & 16 deletions src/Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1412,9 +1412,10 @@ void Task::substitute(const std::string& from, const std::string& to, const std:
// The purpose of Task::validate is three-fold:
// 1) To provide missing attributes where possible
// 2) To provide suitable warnings about odd states
// 3) To generate errors when the inconsistencies are not fixable
// 4) To update status depending on other attributes
// 3) To update status depending on other attributes
//
// As required by TaskChampion, no combination of properties and values is an
// error. This function will try to make sensible defaults and resolve inconsistencies.
// Critically, note that despite the name this is not a read-only function.
//
void Task::validate(bool applyDefault /* = true */) {
Expand All @@ -1428,7 +1429,10 @@ void Task::validate(bool applyDefault /* = true */) {
Lexer lex(uid);
std::string token;
Lexer::Type type;
if (!lex.isUUID(token, type, true)) throw format("Not a valid UUID '{1}'.", uid);
// `uuid` is not a property in the TaskChampion model, so an invalid UUID is
// actually an error.
if (!lex.isUUID(token, type, true))
throw format("Not a valid UUID '{1}'.", uid);
} else
set("uuid", uuid());

Expand Down Expand Up @@ -1543,27 +1547,27 @@ void Task::validate(bool applyDefault /* = true */) {
validate_before("scheduled", "due");
validate_before("scheduled", "end");

// 3) To generate errors when the inconsistencies are not fixable
if (!has("description") || get("description") == "")
Context::getContext().footnote(format("Warning: task has no description."));

// There is no fixing a missing description.
if (!has("description"))
throw std::string("A task must have a description.");
else if (get("description") == "")
throw std::string("Cannot add a task that is blank.");

// Cannot have a recur frequency with no due date - when would it recur?
if (has("recur") && (!has("due") || get("due") == ""))
throw std::string("A recurring task must also have a 'due' date.");
// Cannot have an old-style recur frequency with no due date - when would it recur?
if (has("recur") && (!has("due") || get("due") == "")) {
Context::getContext().footnote(format("Warning: recurring task has no due date."));
remove("recur");
}

// Recur durations must be valid.
// Old-style recur durations must be valid.
if (has("recur")) {
std::string value = get("recur");
if (value != "") {
Duration p;
std::string::size_type i = 0;
if (!p.parse(value, i))
if (!p.parse(value, i)) {
// TODO Ideal location to map unsupported old recurrence periods to supported values.
throw format("The recurrence value '{1}' is not valid.", value);
Context::getContext().footnote(
format("Warning: The recurrence value '{1}' is not valid.", value));
remove("recur");
}
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/commands/CmdImport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ void CmdImport::importSingleTask(json::object* obj) {
// Parse the whole thing, validate the data.
Task task(obj);

// An empty task is probably not intentional - at least a UUID should be included.
if (task.is_empty()) throw format("Cannot import an empty task.");

auto hasGeneratedEntry = not task.has("entry");
auto hasExplicitEnd = task.has("end");

Expand Down
4 changes: 2 additions & 2 deletions test/import.test.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,10 @@ def setUp(self):
self.t = Task()

def test_import_empty_json(self):
"""Verify empty JSON is caught"""
"""Verify empty JSON is ignored"""
j = "{}"
code, out, err = self.t.runError("import", input=j)
self.assertIn("A task must have a description.", err)
self.assertIn("Cannot import an empty task.", err)

def test_import_invalid_uuid(self):
"""Verify invalid UUID is caught"""
Expand Down

0 comments on commit 64f73b4

Please sign in to comment.