diff --git a/doc/man/taskrc.5.in b/doc/man/taskrc.5.in index 96767beea..7ad152ab6 100644 --- a/doc/man/taskrc.5.in +++ b/doc/man/taskrc.5.in @@ -508,15 +508,6 @@ weekly recurring task is added with a due date of tomorrow, and recurrence.limit is set to 2, then a report will list 2 pending recurring tasks, one for tomorrow, and one for a week from tomorrow. -.TP -.B undo.style=side -When the 'undo' command is run, Taskwarrior presents a before and after -comparison of the data. This can be in either the 'side' style, which compares -values side-by-side in a table, or 'diff' style, which uses a format similar to -the 'diff' command. - -Currently not supported. - .TP .B abbreviation.minimum=2 Minimum length of any abbreviated command/value. This means that "ve", "ver", diff --git a/scripts/vim/syntax/taskrc.vim b/scripts/vim/syntax/taskrc.vim index 0e5eb13d0..87cd568e4 100644 --- a/scripts/vim/syntax/taskrc.vim +++ b/scripts/vim/syntax/taskrc.vim @@ -168,7 +168,6 @@ syn match taskrcGoodKey '^\s*\Vsugar='he=e-1 syn match taskrcGoodKey '^\s*\Vsync.\(server.\(url\|origin\|client_id\|encryption_secret\)\|local.server_dir\)='he=e-1 syn match taskrcGoodKey '^\s*\Vtag.indicator='he=e-1 syn match taskrcGoodKey '^\s*\Vuda.\S\{-}.\(default\|type\|label\|values\|indicator\)='he=e-1 -syn match taskrcGoodKey '^\s*\Vundo.style='he=e-1 syn match taskrcGoodKey '^\s*\Vurgency.active.coefficient='he=e-1 syn match taskrcGoodKey '^\s*\Vurgency.age.coefficient='he=e-1 syn match taskrcGoodKey '^\s*\Vurgency.age.max='he=e-1 diff --git a/src/Context.cpp b/src/Context.cpp index 2ba1352fb..9b473b98e 100644 --- a/src/Context.cpp +++ b/src/Context.cpp @@ -127,7 +127,6 @@ std::string configurationDefaults = "dependency.indicator=D # What to show as a dependency indicator\n" "recurrence.indicator=R # What to show as a task recurrence indicator\n" "recurrence.limit=1 # Number of future recurring pending tasks\n" - "undo.style=side # Undo style - can be 'side', or 'diff'\n" "regex=1 # Assume all search/filter strings are " "regexes\n" "xterm.title=0 # Sets xterm title for some commands\n" diff --git a/src/TDB2.cpp b/src/TDB2.cpp index d7fd81017..aab85a8e9 100644 --- a/src/TDB2.cpp +++ b/src/TDB2.cpp @@ -226,107 +226,6 @@ void TDB2::get_changes(std::vector& changes) { [](const auto& kv) { return kv.second; }); } -//////////////////////////////////////////////////////////////////////////////// -void TDB2::revert() { - rust::Vec undo_ops = replica()->get_undo_operations(); - if (undo_ops.size() == 0) { - std::cout << "No operations to undo.\n"; - return; - } - if (confirm_revert(undo_ops)) { - // Note that commit_reversed_operations rebuilds the working set, so that - // need not be done here. - if (!replica()->commit_reversed_operations(std::move(undo_ops))) { - std::cout << "Could not undo: other operations have occurred."; - } - } -} - -//////////////////////////////////////////////////////////////////////////////// -bool TDB2::confirm_revert(rust::Vec& undo_ops) { - // TODO: convert to Operation and use that type for display, similar to CmdInfo. - - // Count non-undo operations - int ops_count = 0; - for (auto& op : undo_ops) { - if (!op.is_undo_point()) { - ops_count++; - } - } - - std::cout << "The following " << ops_count << " operations would be reverted:\n"; - for (auto& op : undo_ops) { - if (op.is_undo_point()) { - continue; - } - - std::cout << "- "; - std::string uuid = static_cast(op.get_uuid().to_string()); - if (op.is_create()) { - std::cout << "Create " << uuid; - } else if (op.is_delete()) { - std::cout << "Delete "; - auto old_task = op.get_old_task(); - bool found_description = false; - for (auto& pv : old_task) { - if (static_cast(pv.prop) == "description") { - std::cout << static_cast(pv.value) << " (" << uuid << ")"; - found_description = true; - } - } - if (!found_description) { - std::cout << uuid; - } - } else if (op.is_update()) { - std::cout << "Update " << uuid << "\n"; - std::string property; - op.get_property(property); - std::string value; - bool have_value = op.get_value(value); - std::string old_value; - bool have_old_value = op.get_old_value(old_value); - std::cout << " " << property << ": "; - std::cout << (have_old_value ? old_value : "") << " -> "; - std::cout << (have_value ? value : ""); - } - std::cout << "\n"; - } - return !Context::getContext().config.getBoolean("confirmation") || - confirm( - "The undo command is not reversible. Are you sure you want to revert to the previous " - "state?"); - return true; -} - -//////////////////////////////////////////////////////////////////////////////// -void TDB2::show_diff(const std::string& current, const std::string& prior, - const std::string& when) { - Datetime lastChange(strtoll(when.c_str(), nullptr, 10)); - - // Set the colors. - Color color_red( - Context::getContext().color() ? Context::getContext().config.get("color.undo.before") : ""); - Color color_green( - Context::getContext().color() ? Context::getContext().config.get("color.undo.after") : ""); - - auto before = prior == "" ? Task() : Task(prior); - auto after = Task(current); - - if (Context::getContext().config.get("undo.style") == "side") { - Table view = before.diffForUndoSide(after); - - std::cout << '\n' - << format("The last modification was made {1}", lastChange.toString()) << '\n' - << '\n' - << view.render() << '\n'; - } - - else if (Context::getContext().config.get("undo.style") == "diff") { - Table view = before.diffForUndoPatch(after, lastChange); - std::cout << '\n' << view.render() << '\n'; - } -} - //////////////////////////////////////////////////////////////////////////////// void TDB2::gc() { Timer timer; diff --git a/src/TDB2.h b/src/TDB2.h index 9de0d9d7e..69ffd0f20 100644 --- a/src/TDB2.h +++ b/src/TDB2.h @@ -51,7 +51,6 @@ class TDB2 { void modify(Task &); void purge(Task &); void get_changes(std::vector &); - void revert(); void gc(); void expire_tasks(); int latest_id(); @@ -75,8 +74,6 @@ class TDB2 { void dump(); - bool confirm_revert(rust::Vec &); - rust::Box &replica(); private: @@ -93,7 +90,6 @@ class TDB2 { const rust::Box &working_set(); void maybe_add_undo_point(rust::Vec &); - static void show_diff(const std::string &, const std::string &, const std::string &); }; #endif diff --git a/src/Task.cpp b/src/Task.cpp index 72cf41975..bab293341 100644 --- a/src/Task.cpp +++ b/src/Task.cpp @@ -2152,188 +2152,3 @@ std::string Task::diff(const Task& after) const { } //////////////////////////////////////////////////////////////////////////////// -// Similar to diff, but formatted as a side-by-side table for an Undo preview -Table Task::diffForUndoSide(const Task& after) const { - // Set the colors. - Color color_red( - Context::getContext().color() ? Context::getContext().config.get("color.undo.before") : ""); - Color color_green( - Context::getContext().color() ? Context::getContext().config.get("color.undo.after") : ""); - - // Attributes are all there is, so figure the different attribute names - // between before and after. - Table view; - view.width(Context::getContext().getWidth()); - view.intraPadding(2); - view.add(""); - view.add("Prior Values"); - view.add("Current Values"); - setHeaderUnderline(view); - - if (!is_empty()) { - const Task& before = *this; - - std::vector beforeAtts; - for (auto& att : before.data) beforeAtts.push_back(att.first); - - std::vector afterAtts; - for (auto& att : after.data) afterAtts.push_back(att.first); - - std::vector beforeOnly; - std::vector afterOnly; - listDiff(beforeAtts, afterAtts, beforeOnly, afterOnly); - - int row; - for (auto& name : beforeOnly) { - row = view.addRow(); - view.set(row, 0, name); - view.set(row, 1, renderAttribute(name, before.get(name)), color_red); - } - - for (auto& att : before.data) { - std::string priorValue = before.get(att.first); - std::string currentValue = after.get(att.first); - - if (currentValue != "") { - row = view.addRow(); - view.set(row, 0, att.first); - view.set(row, 1, renderAttribute(att.first, priorValue), - (priorValue != currentValue ? color_red : Color())); - view.set(row, 2, renderAttribute(att.first, currentValue), - (priorValue != currentValue ? color_green : Color())); - } - } - - for (auto& name : afterOnly) { - row = view.addRow(); - view.set(row, 0, name); - view.set(row, 2, renderAttribute(name, after.get(name)), color_green); - } - } else { - int row; - for (auto& att : after.data) { - row = view.addRow(); - view.set(row, 0, att.first); - view.set(row, 2, renderAttribute(att.first, after.get(att.first)), color_green); - } - } - - return view; -} - -//////////////////////////////////////////////////////////////////////////////// -// Similar to diff, but formatted as a diff for an Undo preview -Table Task::diffForUndoPatch(const Task& after, const Datetime& lastChange) const { - // This style looks like this: - // --- before 2009-07-04 00:00:25.000000000 +0200 - // +++ after 2009-07-04 00:00:45.000000000 +0200 - // - // - name: old // att deleted - // + name: - // - // - name: old // att changed - // + name: new - // - // - name: - // + name: new // att added - // - - // Set the colors. - Color color_red( - Context::getContext().color() ? Context::getContext().config.get("color.undo.before") : ""); - Color color_green( - Context::getContext().color() ? Context::getContext().config.get("color.undo.after") : ""); - - const Task& before = *this; - - // Generate table header. - Table view; - view.width(Context::getContext().getWidth()); - view.intraPadding(2); - view.add(""); - view.add(""); - - int row = view.addRow(); - view.set(row, 0, "--- previous state", color_red); - view.set(row, 1, "Undo will restore this state", color_red); - - row = view.addRow(); - view.set(row, 0, "+++ current state ", color_green); - view.set(row, 1, - format("Change made {1}", - lastChange.toString(Context::getContext().config.get("dateformat"))), - color_green); - - view.addRow(); - - // Add rows to table showing diffs. - std::vector all = Context::getContext().getColumns(); - - // Now factor in the annotation attributes. - for (auto& it : before.data) - if (it.first.substr(0, 11) == "annotation_") all.push_back(it.first); - - for (auto& it : after.data) - if (it.first.substr(0, 11) == "annotation_") all.push_back(it.first); - - // Now render all the attributes. - std::sort(all.begin(), all.end()); - - std::string before_att; - std::string after_att; - std::string last_att; - for (auto& a : all) { - if (a != last_att) // Skip duplicates. - { - last_att = a; - - before_att = before.get(a); - after_att = after.get(a); - - // Don't report different uuid. - // Show nothing if values are the unchanged. - if (a == "uuid" || before_att == after_att) { - // Show nothing - no point displaying that which did not change. - - // row = view.addRow (); - // view.set (row, 0, *a + ":"); - // view.set (row, 1, before_att); - } - - // Attribute deleted. - else if (before_att != "" && after_att == "") { - row = view.addRow(); - view.set(row, 0, '-' + a + ':', color_red); - view.set(row, 1, before_att, color_red); - - row = view.addRow(); - view.set(row, 0, '+' + a + ':', color_green); - } - - // Attribute added. - else if (before_att == "" && after_att != "") { - row = view.addRow(); - view.set(row, 0, '-' + a + ':', color_red); - - row = view.addRow(); - view.set(row, 0, '+' + a + ':', color_green); - view.set(row, 1, after_att, color_green); - } - - // Attribute changed. - else { - row = view.addRow(); - view.set(row, 0, '-' + a + ':', color_red); - view.set(row, 1, before_att, color_red); - - row = view.addRow(); - view.set(row, 0, '+' + a + ':', color_green); - view.set(row, 1, after_att, color_green); - } - } - } - - return view; -} - -//////////////////////////////////////////////////////////////////////////////// diff --git a/src/Task.h b/src/Task.h index 1f63b7f5a..66e18b13b 100644 --- a/src/Task.h +++ b/src/Task.h @@ -181,8 +181,6 @@ class Task { #endif std::string diff(const Task& after) const; - Table diffForUndoSide(const Task& after) const; - Table diffForUndoPatch(const Task& after, const Datetime& lastChange) const; private: int determineVersion(const std::string&); diff --git a/src/commands/CmdShow.cpp b/src/commands/CmdShow.cpp index bacc5c464..df92df6a1 100644 --- a/src/commands/CmdShow.cpp +++ b/src/commands/CmdShow.cpp @@ -202,7 +202,6 @@ int CmdShow::execute(std::string& output) { " sync.server.url" " sync.server.origin" " tag.indicator" - " undo.style" " urgency.active.coefficient" " urgency.scheduled.coefficient" " urgency.annotations.coefficient" diff --git a/src/commands/CmdUndo.cpp b/src/commands/CmdUndo.cpp index b462ae590..a5257ab93 100644 --- a/src/commands/CmdUndo.cpp +++ b/src/commands/CmdUndo.cpp @@ -29,6 +29,13 @@ #include #include +#include +#include + +#include +#include + +#include "shared.h" //////////////////////////////////////////////////////////////////////////////// CmdUndo::CmdUndo() { @@ -47,8 +54,94 @@ CmdUndo::CmdUndo() { //////////////////////////////////////////////////////////////////////////////// int CmdUndo::execute(std::string&) { - Context::getContext().tdb2.revert(); + auto& replica = Context::getContext().tdb2.replica(); + rust::Vec undo_ops = replica->get_undo_operations(); + if (confirm_revert(Operation::operations(undo_ops))) { + // Note that commit_reversed_operations rebuilds the working set, so that + // need not be done here. + if (!replica->commit_reversed_operations(std::move(undo_ops))) { + std::cout << "Could not undo: other operations have occurred."; + } + } return 0; } //////////////////////////////////////////////////////////////////////////////// +bool CmdUndo::confirm_revert(const std::vector& undo_ops) { + // Count non-undo operations + int ops_count = 0; + for (auto& op : undo_ops) { + if (!op.is_undo_point()) { + ops_count++; + } + } + if (ops_count == 0) { + std::cout << "No operations to undo.\n"; + return true; + } + + std::cout << "The following " << ops_count << " operations would be reverted:\n"; + + Table view; + if (Context::getContext().config.getBoolean("obfuscate")) view.obfuscate(); + view.width(Context::getContext().getWidth()); + view.add("Uuid"); + view.add("Modification"); + + std::string last_uuid; + std::stringstream mods; + for (auto& op : undo_ops) { + if (op.is_undo_point()) { + continue; + } + + if (last_uuid != op.get_uuid()) { + if (last_uuid.size() != 0) { + int row = view.addRow(); + view.set(row, 0, last_uuid); + view.set(row, 1, mods.str()); + } + last_uuid = op.get_uuid(); + mods.clear(); + } + + if (op.is_create()) { + mods << "Create task\n"; + } else if (op.is_delete()) { + mods << "Delete (purge) task"; + } else if (op.is_update()) { + auto property = op.get_property(); + auto old_value = op.get_old_value(); + auto value = op.get_value(); + if (Task::isTagAttr(property)) { + if (value && *value == "x") { + mods << "Add tag '" << Task::attr2Tag(property) << "'\n"; + continue; + } else if (!value && old_value && *old_value == "x") { + mods << "Remove tag '" << Task::attr2Tag(property) << "'\n"; + continue; + } + } + if (old_value && value) { + mods << "Update property '" << property << "' from '" << *old_value << "' to '" << *value + << "'\n"; + } else if (old_value) { + mods << "Delete property '" << property << "' (was '" << *old_value << "')\n"; + } else if (value) { + mods << "Add property '" << property << "' with value '" << *value << "'\n"; + } + } + } + int row = view.addRow(); + view.set(row, 0, last_uuid); + view.set(row, 1, mods.str()); + std::cout << view.render() << "\n"; + + return !Context::getContext().config.getBoolean("confirmation") || + confirm( + "The undo command is not reversible. Are you sure you want to revert to the previous " + "state?"); + return true; +} + +//////////////////////////////////////////////////////////////////////////////// diff --git a/src/commands/CmdUndo.h b/src/commands/CmdUndo.h index 40993311f..590184178 100644 --- a/src/commands/CmdUndo.h +++ b/src/commands/CmdUndo.h @@ -28,13 +28,18 @@ #define INCLUDED_CMDUNDO #include +#include #include +#include class CmdUndo : public Command { public: CmdUndo(); - int execute(std::string&); + int execute(std::string &); + + private: + bool confirm_revert(const std::vector &); }; #endif diff --git a/test/undo.test.py b/test/undo.test.py index b0beca1b1..25d5dd4ba 100755 --- a/test/undo.test.py +++ b/test/undo.test.py @@ -74,33 +74,13 @@ def setUp(self): self.t("add one project:foo priority:H") self.t("1 modify +tag project:bar priority:") - @unittest.expectedFailure # undo diffs are not supported - def test_undo_side_style(self): - """Test that 'rc.undo.style:side' generates the right output""" - self.t.config("undo.style", "side") + def test_undo_output(self): + """Test that 'task undo' generates the right output""" code, out, err = self.t("undo", input="n\n") self.assertNotRegex(out, "-tags:\\s*\n\\+tags:\\s+tag") - self.assertRegex(out, r"tags\s+tag\s*") - - @unittest.expectedFailure # undo diffs are not supported - def test_undo_diff_style(self): - """Test that 'rc.undo.style:diff' generates the right output""" - self.t.config("undo.style", "diff") - code, out, err = self.t("undo", input="n\n") - self.assertRegex(out, "-tags:\\s*\n\\+tags:\\s+tag") - self.assertNotRegex(out, r"tags\s+tag\s*") - - def test_undo_diff_operations(self): - code, out, err = self.t("undo", input="n\n") - - # If the clock ticks a second between `add` and `modify` there is a - # fifth operation setting the `modified` property. - self.assertRegex(out, "The following [4|5] operations would be reverted:") - - self.assertIn("tag_tag: -> x", out) - self.assertIn("tags: -> tag", out) - self.assertIn("project: foo -> bar", out) - self.assertIn("priority: H -> ", out) + self.assertRegex(out, r"Delete property 'priority'"); + self.assertRegex(out, r"Update property 'project'"); + self.assertRegex(out, r"Add tag 'tag'"); class TestBug634(TestCase):