mirror of
https://github.com/GothenburgBitFactory/taskwarrior.git
synced 2025-06-26 10:54:26 +02:00
Better undo output (and remove undo.style config) (#3672)
This commit is contained in:
Dustin J. Mitchell
GitHub
parent
dcc8a8cdde
commit
a2f9b92d6c
11 changed files with 105 additions and 331 deletions
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
101
src/TDB2.cpp
101
src/TDB2.cpp
|
|
@ -226,107 +226,6 @@ void TDB2::get_changes(std::vector<Task>& changes) {
|
|||
[](const auto& kv) { return kv.second; });
|
||||
}
|
||||
|
||||
////////////////////////////////////////////////////////////////////////////////
|
||||
void TDB2::revert() {
|
||||
rust::Vec<tc::Operation> 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<tc::Operation>& 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<std::string>(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<std::string>(pv.prop) == "description") {
|
||||
std::cout << static_cast<std::string>(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 : "<empty>") << " -> ";
|
||||
std::cout << (have_value ? value : "<empty>");
|
||||
}
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -51,7 +51,6 @@ class TDB2 {
|
|||
void modify(Task &);
|
||||
void purge(Task &);
|
||||
void get_changes(std::vector<Task> &);
|
||||
void revert();
|
||||
void gc();
|
||||
void expire_tasks();
|
||||
int latest_id();
|
||||
|
|
@ -75,8 +74,6 @@ class TDB2 {
|
|||
|
||||
void dump();
|
||||
|
||||
bool confirm_revert(rust::Vec<tc::Operation> &);
|
||||
|
||||
rust::Box<tc::Replica> &replica();
|
||||
|
||||
private:
|
||||
|
|
@ -93,7 +90,6 @@ class TDB2 {
|
|||
|
||||
const rust::Box<tc::WorkingSet> &working_set();
|
||||
void maybe_add_undo_point(rust::Vec<tc::Operation> &);
|
||||
static void show_diff(const std::string &, const std::string &, const std::string &);
|
||||
};
|
||||
|
||||
#endif
|
||||
|
|
|
|||
185
src/Task.cpp
185
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<std::string> beforeAtts;
|
||||
for (auto& att : before.data) beforeAtts.push_back(att.first);
|
||||
|
||||
std::vector<std::string> afterAtts;
|
||||
for (auto& att : after.data) afterAtts.push_back(att.first);
|
||||
|
||||
std::vector<std::string> beforeOnly;
|
||||
std::vector<std::string> 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<std::string> 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;
|
||||
}
|
||||
|
||||
////////////////////////////////////////////////////////////////////////////////
|
||||
|
|
|
|||
|
|
@ -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&);
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -29,6 +29,13 @@
|
|||
|
||||
#include <CmdUndo.h>
|
||||
#include <Context.h>
|
||||
#include <Operation.h>
|
||||
#include <Task.h>
|
||||
|
||||
#include <iostream>
|
||||
#include <sstream>
|
||||
|
||||
#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<tc::Operation> 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<Operation>& 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;
|
||||
}
|
||||
|
||||
////////////////////////////////////////////////////////////////////////////////
|
||||
|
|
|
|||
|
|
@ -28,13 +28,18 @@
|
|||
#define INCLUDED_CMDUNDO
|
||||
|
||||
#include <Command.h>
|
||||
#include <Operation.h>
|
||||
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
class CmdUndo : public Command {
|
||||
public:
|
||||
CmdUndo();
|
||||
int execute(std::string&);
|
||||
int execute(std::string &);
|
||||
|
||||
private:
|
||||
bool confirm_revert(const std::vector<Operation> &);
|
||||
};
|
||||
|
||||
#endif
|
||||
|
|
|
|||
|
|
@ -74,33 +74,13 @@ class TestUndoStyle(TestCase):
|
|||
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: <empty> -> x", out)
|
||||
self.assertIn("tags: <empty> -> tag", out)
|
||||
self.assertIn("project: foo -> bar", out)
|
||||
self.assertIn("priority: H -> <empty>", 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):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue