diff --git a/src/Task.cpp b/src/Task.cpp index 894a07e27..e7c6c377a 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -630,6 +630,15 @@ void Task::parse (const std::string& input) parseLegacy (input); } + // for compatibility, include all tags in `tags` as `tag_..` attributes + if (data.find ("tags") != data.end ()) { + for (auto& tag : split(data["tags"], ',')) { + data[tag2Attr(tag)] = "x"; + } + } + // ..and similarly, update `tags` to match the `tag_..` attributes + fixTagsAttribute(); + recalc_urgency = true; } @@ -692,16 +701,6 @@ void Task::parseJSON (const json::object* root_obj) addTag (tag->_data); } } - // This is a temporary measure to accomodate a malformed JSON message from - // Mirakel sync. - // - // 2016-02-21 Mirakel dropped sync support in late 2015. This can be - // removed in a later release. - else if (i.first == "tags" && i.second->type() == json::j_string) - { - auto tag = (json::string*)i.second; - addTag (tag->_data); - } // Dependencies can be exported as an array of strings. // 2016-02-21: This will be the only option in future releases. @@ -906,6 +905,10 @@ std::string Task::composeJSON (bool decorate /*= false*/) const if (! i.first.compare (0, 11, "annotation_", 11)) continue; + if (i.first == "tags" || isTagAttr (i.first)) + // Tags are handled below + continue; + // If value is an empty string, do not ever output it if (i.second == "") continue; @@ -947,26 +950,6 @@ std::string Task::composeJSON (bool decorate /*= false*/) const ++attributes_written; } - // Tags are converted to an array. - else if (i.first == "tags") - { - auto tags = split (i.second, ','); - - out << "\"tags\":["; - - int count = 0; - for (const auto& i : tags) - { - if (count++) - out << ','; - - out << '"' << i << '"'; - } - - out << ']'; - ++attributes_written; - } - // Dependencies are an array by default. else if (i.first == "depends" #ifdef PRODUCT_TASKWARRIOR @@ -1047,6 +1030,25 @@ std::string Task::composeJSON (bool decorate /*= false*/) const out << ']'; } + auto tags = getTags(); + if (tags.size() > 0) + { + out << ',' + << "\"tags\":["; + + int count = 0; + for (const auto& tag : tags) + { + if (count++) + out << ','; + + out << '"' << tag << '"'; + } + + out << ']'; + ++attributes_written; + } + #ifdef PRODUCT_TASKWARRIOR // Include urgency. if (decorate) @@ -1257,8 +1259,13 @@ std::vector Task::getDependencyTasks () const //////////////////////////////////////////////////////////////////////////////// int Task::getTagCount () const { - auto tags = split (get ("tags"), ','); - return (int) tags.size (); + auto count = 0; + for (auto& attr : data) { + if (isTagAttr (attr.first)) { + count++; + } + } + return count; } //////////////////////////////////////////////////////////////////////////////// @@ -1301,7 +1308,7 @@ bool Task::hasTag (const std::string& tag) const if (tag == "INSTANCE") return has ("template") || has ("parent"); if (tag == "UNTIL") return has ("until"); if (tag == "ANNOTATED") return hasAnnotations (); - if (tag == "TAGGED") return has ("tags"); + if (tag == "TAGGED") return getTagCount() > 0; if (tag == "PARENT") return has ("mask") || has ("last"); // 2017-01-07: Deprecated in 2.6.0 if (tag == "TEMPLATE") return has ("last") || has ("mask"); if (tag == "WAITING") return get ("status") == "waiting"; @@ -1318,9 +1325,7 @@ bool Task::hasTag (const std::string& tag) const } // Concrete tags. - auto tags = split (get ("tags"), ','); - - if (std::find (tags.begin (), tags.end (), tag) != tags.end ()) + if (has (tag2Attr (tag))) return true; return false; @@ -1329,47 +1334,104 @@ bool Task::hasTag (const std::string& tag) const //////////////////////////////////////////////////////////////////////////////// void Task::addTag (const std::string& tag) { - auto tags = split (get ("tags"), ','); - - if (std::find (tags.begin (), tags.end (), tag) == tags.end ()) - { - tags.push_back (tag); - set ("tags", join (",", tags)); - + auto attr = tag2Attr (tag); + if (!has (attr)) { + set (attr, "x"); recalc_urgency = true; + fixTagsAttribute(); } } //////////////////////////////////////////////////////////////////////////////// -void Task::addTags (const std::vector & tags) +void Task::setTags (const std::vector & tags) { - remove ("tags"); + auto existing = getTags(); - for (auto& tag : tags) + // edit in-place, determining which should be + // added and which should be removed + std::vector toAdd; + std::vector toRemove; + + for (auto& tag : tags) { + if (std::find (existing.begin (), existing.end (), tag) == existing.end ()) + toAdd.push_back(tag); + } + + for (auto& tag : getTags ()) { + if (std::find (tags.begin (), tags.end (), tag) == tags.end ()) { + toRemove.push_back (tag); + } + } + + for (auto& tag : toRemove) { + removeTag (tag); + } + for (auto& tag : toAdd) { addTag (tag); + } - recalc_urgency = true; + // (note: addTag / removeTag took care of recalculating urgency) } //////////////////////////////////////////////////////////////////////////////// std::vector Task::getTags () const { - return split (get ("tags"), ','); + std::vector tags; + + for (auto& attr : data) { + if (!isTagAttr (attr.first)) { + continue; + } + auto tag = attr2Tag (attr.first); + tags.push_back (tag); + } + + return tags; } //////////////////////////////////////////////////////////////////////////////// void Task::removeTag (const std::string& tag) { - auto tags = split (get ("tags"), ','); - - auto i = std::find (tags.begin (), tags.end (), tag); - if (i != tags.end ()) - { - tags.erase (i); - set ("tags", join (",", tags)); + auto attr = tag2Attr (tag); + if (has (attr)) { + data.erase (attr); + recalc_urgency = true; + fixTagsAttribute(); } +} - recalc_urgency = true; +//////////////////////////////////////////////////////////////////////////////// +void Task::fixTagsAttribute () +{ + // Fix up the old `tags` attribute to match the `tags_..` attributes (or + // remove it if there are no tags) + auto tags = getTags (); + if (tags.size () > 0) { + set ("tags", join (",", tags)); + } else { + remove ("tags"); + } +} + +//////////////////////////////////////////////////////////////////////////////// +bool Task::isTagAttr(const std::string& attr) const +{ + return attr.compare(0, 5, "tags_") == 0; +} + +//////////////////////////////////////////////////////////////////////////////// +const std::string Task::tag2Attr (const std::string& tag) const +{ + std::stringstream tag_attr; + tag_attr << "tags_" << tag; + return tag_attr.str(); +} + +//////////////////////////////////////////////////////////////////////////////// +const std::string Task::attr2Tag (const std::string& attr) const +{ + assert (isTagAttr (attr)); + return attr.substr(5); } #ifdef PRODUCT_TASKWARRIOR diff --git a/src/Task.h b/src/Task.h index 49bc4e019..5112dc3f1 100644 --- a/src/Task.h +++ b/src/Task.h @@ -125,7 +125,7 @@ public: int getTagCount () const; bool hasTag (const std::string&) const; void addTag (const std::string&); - void addTags (const std::vector &); + void setTags (const std::vector &); std::vector getTags () const; void removeTag (const std::string&); @@ -170,6 +170,10 @@ private: void validate_before (const std::string&, const std::string&); const std::string encode (const std::string&) const; const std::string decode (const std::string&) const; + bool isTagAttr (const std::string&) const; + const std::string tag2Attr (const std::string&) const; + const std::string attr2Tag (const std::string&) const; + void fixTagsAttribute (); public: float urgency_project () const; diff --git a/src/columns/ColTags.cpp b/src/columns/ColTags.cpp index be1336dc7..97dc41d82 100644 --- a/src/columns/ColTags.cpp +++ b/src/columns/ColTags.cpp @@ -115,17 +115,16 @@ void ColumnTags::render ( int width, Color& color) { - if (task.has (_name)) + auto all = task.getTags (); + if (all.size() > 0) { - std::string tags = task.get (_name); if (_style == "default" || _style == "list") { - if (tags.find (',') != std::string::npos) + if (all.size () > 1) { - auto all = split (tags, ','); std::sort (all.begin (), all.end ()); - tags = join (" ", all); + auto tags = join (" ", all); all.clear (); wrapText (all, tags, width, _hyphenate); @@ -134,7 +133,7 @@ void ColumnTags::render ( renderStringLeft (lines, width, color, i); } else - renderStringLeft (lines, width, color, tags); + renderStringLeft (lines, width, color, all[0]); } else if (_style == "indicator") { @@ -142,7 +141,6 @@ void ColumnTags::render ( } else if (_style == "count") { - auto all = split (tags, ','); renderStringRight (lines, width, color, '[' + format (static_cast (all.size ())) + ']'); } } @@ -152,36 +150,36 @@ void ColumnTags::render ( void ColumnTags::modify (Task& task, const std::string& value) { std::string label = " MODIFICATION "; + std::string commasep; + std::vector tags; - // TW-1701 - task.set (_name, ""); - - for (auto& tag : split (value, ',')) + // If it's a DOM ref, eval it first. + Lexer lexer (value); + std::string domRef; + Lexer::Type type; + if (lexer.token (domRef, type) && + type == Lexer::Type::dom) { - // If it's a DOM ref, eval it first. - Lexer lexer (tag); - std::string domRef; - Lexer::Type type; - if (lexer.token (domRef, type) && - type == Lexer::Type::dom) - { - Eval e; - e.addSource (domSource); - contextTask = task; + Eval e; + e.addSource (domSource); + contextTask = task; - Variant v; - e.evaluateInfixExpression (value, v); - task.addTag ((std::string) v); - Context::getContext ().debug (label + "tags <-- '" + (std::string) v + "' <-- '" + tag + '\''); - } - else - { - task.addTag (tag); - Context::getContext ().debug (label + "tags <-- '" + tag + '\''); - } + Variant v; + e.evaluateInfixExpression (value, v); + commasep = (std::string) v; + } else { + commasep = (std::string) value; + } + + for (auto& tag : split (commasep, ',')) + { + tags.push_back ((std::string) tag); + Context::getContext ().debug (label + "tags <-- '" + tag + '\''); feedback_special_tags (task, tag); } + + task.setTags(tags); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/commands/CmdEdit.cpp b/src/commands/CmdEdit.cpp index 3751b5e6d..0c69e4faa 100644 --- a/src/commands/CmdEdit.cpp +++ b/src/commands/CmdEdit.cpp @@ -366,7 +366,7 @@ void CmdEdit::parseTask (Task& task, const std::string& after, const std::string // tags value = findValue (after, "\n Tags:"); task.remove ("tags"); - task.addTags (split (value, ' ')); + task.setTags (split (value, ' ')); // description. value = findMultilineValue (after, "\n Description:", "\n Created:"); diff --git a/src/commands/CmdInfo.cpp b/src/commands/CmdInfo.cpp index 7b8d7da22..0d719a2d7 100644 --- a/src/commands/CmdInfo.cpp +++ b/src/commands/CmdInfo.cpp @@ -413,6 +413,7 @@ int CmdInfo::execute (std::string& output) for (auto& att : all) { if (att.substr (0, 11) != "annotation_" && + att.substr (0, 5) != "tags_" && Context::getContext ().columns.find (att) == Context::getContext ().columns.end ()) { row = view.addRow (); diff --git a/test/tag.t b/test/tag.t index 39f7ea609..4ff267c1d 100755 --- a/test/tag.t +++ b/test/tag.t @@ -44,11 +44,16 @@ class TestTags(TestCase): def setUp(self): """Executed before each test in the class""" + def split_tags(self, tags): + return sorted(tags.strip().split(',')) + def test_tag_manipulation(self): """Test addition and removal of tags""" self.t("add +one This +two is a test +three") code, out, err = self.t("_get 1.tags") - self.assertEqual("one,two,three\n", out) + self.assertEqual( + sorted(["one", "two", "three"]), + self.split_tags(out)) # Remove tags. self.t("1 modify -three -two -one") @@ -58,7 +63,9 @@ class TestTags(TestCase): # Add tags. self.t("1 modify +four +five +six") code, out, err = self.t("_get 1.tags") - self.assertEqual("four,five,six\n", out) + self.assertEqual( + sorted(["four", "five", "six"]), + self.split_tags(out)) # Remove tags. self.t("1 modify -four -five -six")