Refactor to store tags as individual attributes

Each tag is stored as `tag_<tagname>: x`.  The `x` is required because
empty attributes are treated as nonexistent.

For compatibility, the `tags` attribute is updated in sync with the
per-tag attributes.  This compatibility support may be dropped in later
versions.

Note that synchronization _updates_ use JSON format, which does not
change with this patch, and thus no compatibility issues exist.  The
synchronization _initialization_, however, uses FF4, meaning that a
sync server initialized from a version of `task` with this patch will
contain `tag_<tagname>` attributes, which will look like orphaned UDAs
to older versions.  However, as updates to tasks are synchronized via
the sync server, the updates will not contain these attributes and they
will show as "deleted" in the `task info` display on the older version.
Aside from the noise in the `task info` output, this is harmless.
This commit is contained in:
Dustin J. Mitchell 2021-08-02 01:25:16 +00:00 committed by Tomas Babej
parent 17e6257e07
commit 20041c120e
6 changed files with 163 additions and 91 deletions

View file

@ -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> 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 <std::string>& tags)
void Task::setTags (const std::vector <std::string>& 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 <std::string> toAdd;
std::vector <std::string> 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 <std::string> Task::getTags () const
{
return split (get ("tags"), ',');
std::vector <std::string> 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();
}
}
////////////////////////////////////////////////////////////////////////////////
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

View file

@ -125,7 +125,7 @@ public:
int getTagCount () const;
bool hasTag (const std::string&) const;
void addTag (const std::string&);
void addTags (const std::vector <std::string>&);
void setTags (const std::vector <std::string>&);
std::vector <std::string> 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;

View file

@ -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 <int> (all.size ())) + ']');
}
}
@ -152,14 +150,11 @@ void ColumnTags::render (
void ColumnTags::modify (Task& task, const std::string& value)
{
std::string label = " MODIFICATION ";
std::string commasep;
std::vector <std::string> tags;
// TW-1701
task.set (_name, "");
for (auto& tag : split (value, ','))
{
// If it's a DOM ref, eval it first.
Lexer lexer (tag);
Lexer lexer (value);
std::string domRef;
Lexer::Type type;
if (lexer.token (domRef, type) &&
@ -171,17 +166,20 @@ void ColumnTags::modify (Task& task, const std::string& value)
Variant v;
e.evaluateInfixExpression (value, v);
task.addTag ((std::string) v);
Context::getContext ().debug (label + "tags <-- '" + (std::string) v + "' <-- '" + tag + '\'');
commasep = (std::string) v;
} else {
commasep = (std::string) value;
}
else
for (auto& tag : split (commasep, ','))
{
task.addTag (tag);
tags.push_back ((std::string) tag);
Context::getContext ().debug (label + "tags <-- '" + tag + '\'');
}
feedback_special_tags (task, tag);
}
task.setTags(tags);
}
////////////////////////////////////////////////////////////////////////////////

View file

@ -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:");

View file

@ -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 ();

View file

@ -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")