Move history annotation to History#perform/replace

Actions are too low-level for annotations.
This commit is contained in:
John Firebaugh
2012-12-07 10:32:44 -05:00
parent 7eec007740
commit 3e45afbc5e
21 changed files with 142 additions and 114 deletions

View File

@@ -1,6 +1,6 @@
// https://github.com/openstreetmap/josm/blob/mirror/src/org/openstreetmap/josm/command/AddCommand.java
iD.actions.AddNode = function(node) {
return function(graph) {
return graph.replace(node, 'added a place');
return graph.replace(node);
};
};

View File

@@ -1,5 +1,5 @@
iD.actions.AddWay = function(way) {
return function(graph) {
return graph.replace(way, 'started a road');
return graph.replace(way);
};
};

View File

@@ -5,6 +5,6 @@ iD.actions.AddWayNode = function(wayId, nodeId, index) {
node = graph.entity(nodeId),
nodes = way.nodes.slice();
nodes.splice(index || nodes.length, 0, nodeId);
return graph.replace(way.update({nodes: nodes}), 'added to a road');
return graph.replace(way.update({nodes: nodes}));
};
};

View File

@@ -1,6 +1,6 @@
iD.actions.ChangeEntityTags = function(entityId, tags) {
return function(graph) {
var entity = graph.entity(entityId);
return graph.replace(entity.update({tags: tags}), 'changed tags');
return graph.replace(entity.update({tags: tags}));
};
};

View File

@@ -13,6 +13,6 @@ iD.actions.DeleteNode = function(nodeId) {
graph = iD.actions.RemoveRelationMember(parent.id, nodeId)(graph);
});
return graph.remove(node, 'removed a node');
return graph.remove(node);
};
};

View File

@@ -26,6 +26,6 @@ iD.actions.DeleteWay = function(wayId) {
}
});
return graph.remove(way, 'removed a way');
return graph.remove(way);
};
};

View File

@@ -3,6 +3,6 @@
iD.actions.Move = function(entityId, loc) {
return function(graph) {
var entity = graph.entity(entityId);
return graph.replace(entity.update({loc: loc}), 'moved an element');
return graph.replace(entity.update({loc: loc}));
};
};

View File

@@ -2,6 +2,6 @@ iD.actions.RemoveRelationMember = function(relationId, memberId) {
return function(graph) {
var relation = graph.entity(relationId),
members = _.without(relation.members, memberId);
return graph.replace(relation.update({members: members}), 'removed from a relation');
return graph.replace(relation.update({members: members}));
};
};

View File

@@ -12,6 +12,6 @@ iD.actions.RemoveWayNode = function(wayId, nodeId) {
} else {
nodes = _.without(way.nodes, nodeId);
}
return graph.replace(way.update({nodes: nodes}), 'removed from a road');
return graph.replace(way.update({nodes: nodes}));
};
};

View File

@@ -3,6 +3,6 @@ iD.actions.ReverseWay = function(wayId) {
return function(graph) {
var way = graph.entity(wayId),
nodes = way.nodes.slice().reverse();
return graph.replace(way.update({nodes: nodes}), 'changed way direction');
return graph.replace(way.update({nodes: nodes}));
};
};

View File

@@ -1,5 +1,5 @@
iD.Graph = function(entities, annotation) {
if (!(this instanceof iD.Graph)) return new iD.Graph(entities, annotation);
iD.Graph = function(entities) {
if (!(this instanceof iD.Graph)) return new iD.Graph(entities);
if (_.isArray(entities)) {
this.entities = {};
@@ -10,8 +10,6 @@ iD.Graph = function(entities, annotation) {
this.entities = entities || {};
}
this.annotation = annotation;
if (iD.debug) {
Object.freeze(this);
Object.freeze(this.entities);
@@ -40,19 +38,19 @@ iD.Graph.prototype = {
merge: function(graph) {
var entities = _.clone(this.entities);
_.defaults(entities, graph.entities);
return iD.Graph(entities, this.annotation);
return iD.Graph(entities);
},
replace: function(entity, annotation) {
replace: function(entity) {
var entities = _.clone(this.entities);
entities[entity.id] = entity;
return iD.Graph(entities, annotation);
return iD.Graph(entities);
},
remove: function(entity, annotation) {
remove: function(entity) {
var entities = _.clone(this.entities);
delete entities[entity.id];
return iD.Graph(entities, annotation);
return iD.Graph(entities);
},
// get all objects that intersect an extent.

View File

@@ -2,6 +2,23 @@ iD.History = function() {
var stack, index,
dispatch = d3.dispatch('change');
function perform(actions) {
actions = Array.prototype.slice.call(actions)
var annotation;
if (_.isString(_.last(actions))) {
annotation = actions.pop();
}
var graph = stack[index].graph;
for (var i = 0; i < actions.length; i++) {
graph = actions[i](graph);
}
return {graph: graph, annotation: annotation};
}
function maybeChange() {
if (stack[index].annotation) {
dispatch.change();
@@ -10,38 +27,26 @@ iD.History = function() {
var history = {
graph: function () {
return stack[index];
return stack[index].graph;
},
merge: function (graph) {
for (var i = 0; i < stack.length; i++) {
stack[i] = stack[i].merge(graph);
stack[i].graph = stack[i].graph.merge(graph);
}
},
perform: function () {
stack = stack.slice(0, index + 1);
var graph = this.graph();
for (var i = 0; i < arguments.length; i++) {
graph = arguments[i](graph);
}
stack.push(graph);
stack.push(perform(arguments));
index++;
maybeChange();
dispatch.change();
},
replace: function () {
// assert(index == stack.length - 1)
var graph = this.graph();
for (var i = 0; i < arguments.length; i++) {
graph = arguments[i](graph);
}
stack[index] = graph;
maybeChange();
stack[index] = perform(arguments);
dispatch.change();
},
undo: function () {
@@ -78,19 +83,19 @@ iD.History = function() {
// generate reports of changes for changesets to use
modify: function () {
return stack[index].modifications();
return stack[index].graph.modifications();
},
create: function () {
return stack[index].creations();
return stack[index].graph.creations();
},
'delete': function () {
return _.difference(
_.pluck(stack[0].entities, 'id'),
_.pluck(stack[index].entities, 'id')
_.pluck(stack[0].graph.entities, 'id'),
_.pluck(stack[index].graph.entities, 'id')
).map(function (id) {
return stack[0].fetch(id);
return stack[0].graph.fetch(id);
});
},
@@ -103,7 +108,7 @@ iD.History = function() {
},
reset: function () {
stack = [iD.Graph()];
stack = [{graph: iD.Graph()}];
index = 0;
dispatch.change();
}

View File

@@ -22,14 +22,17 @@ iD.modes.AddArea = function() {
// start from an existing node
history.perform(
iD.actions.AddWay(way),
iD.actions.AddWayNode(way.id, datum.id));
iD.actions.AddWayNode(way.id, datum.id),
'started an area');
} else {
// start from a new node
var node = iD.Node({loc: map.mouseCoordinates()});
history.perform(
iD.actions.AddWay(way),
iD.actions.AddNode(node),
iD.actions.AddWayNode(way.id, node.id));
iD.actions.AddWayNode(way.id, node.id),
'started an area');
}
controller.enter(iD.modes.DrawArea(way.id));

View File

@@ -14,7 +14,11 @@ iD.modes.AddPlace = function() {
map.surface.on('click.addplace', function() {
var node = iD.Node({loc: map.mouseCoordinates(), _poi: true});
history.perform(iD.actions.AddNode(node));
history.perform(
iD.actions.AddNode(node),
'added a place');
controller.enter(iD.modes.Select(node));
});

View File

@@ -31,8 +31,10 @@ iD.modes.AddRoad = function() {
} else {
history.perform(
iD.actions.AddWay(way),
iD.actions.AddWayNode(way.id, datum.id));
iD.actions.AddWayNode(way.id, datum.id),
'started a road');
}
} else if (datum.type === 'way') {
// begin a new way starting from an existing way
var node = iD.Node({loc: map.mouseCoordinates()}),
@@ -41,7 +43,9 @@ iD.modes.AddRoad = function() {
history.perform(
iD.actions.AddWay(way),
iD.actions.AddWayNode(datum.id, node, index),
iD.actions.AddWayNode(way.id, node.id));
iD.actions.AddWayNode(way.id, node.id),
'started a road');
} else {
// begin a new way
var node = iD.Node({loc: map.mouseCoordinates()});
@@ -49,7 +53,8 @@ iD.modes.AddRoad = function() {
history.perform(
iD.actions.AddWay(way),
iD.actions.AddNode(node),
iD.actions.AddWayNode(way.id, node.id));
iD.actions.AddWayNode(way.id, node.id),
'started a road');
}
controller.enter(iD.modes.DrawRoad(way.id, direction));

View File

@@ -1,5 +1,5 @@
iD.modes._dragFeatures = function(mode) {
var dragging, incarnated;
var dragging;
var dragbehavior = d3.behavior.drag()
.origin(function(entity) {
@@ -8,26 +8,31 @@ iD.modes._dragFeatures = function(mode) {
})
.on('drag', function(entity) {
d3.event.sourceEvent.stopPropagation();
var loc = mode.map.projection.invert([d3.event.x, d3.event.y]);
if (!dragging) {
if (entity.accuracy) {
var node = iD.Node({ loc: entity.loc });
dragging = iD.Node({loc: loc});
mode.history.perform(
iD.actions.AddNode(node),
iD.actions.AddWayNode(entity.way, node.id, entity.index));
incarnated = node.id;
iD.actions.AddNode(dragging),
iD.actions.AddWayNode(entity.way, dragging.id, entity.index));
} else {
dragging = entity;
mode.history.perform(
iD.actions.Move(dragging.id, loc));
}
dragging = iD.util.trueObj([entity.id].concat(
_.pluck(mode.history.graph().parentWays(entity.id), 'id')));
mode.history.perform(iD.actions.Noop());
}
if (incarnated) entity = mode.history.graph().entity(incarnated);
var to = mode.map.projection.invert([d3.event.x, d3.event.y]);
mode.history.replace(iD.actions.Move(entity.id, to));
mode.history.replace(iD.actions.Move(dragging.id, loc));
})
.on('dragend', function () {
.on('dragend', function (entity) {
if (!dragging) return;
dragging = undefined;
incarnated = undefined;
mode.history.replace(
iD.actions.Noop(),
entity.accuracy ? 'added a node to a way' : 'moved a node');
});
mode.map.surface

View File

@@ -31,7 +31,8 @@ iD.modes.DrawArea = function(wayId) {
if (datum.id === tailId) {
history.replace(
iD.actions.DeleteNode(node.id),
iD.actions.AddWayNode(way.id, tailId));
iD.actions.AddWayNode(way.id, tailId),
'added to an area');
controller.enter(iD.modes.Select(way));
@@ -39,11 +40,16 @@ iD.modes.DrawArea = function(wayId) {
// connect the way to an existing node
history.replace(
iD.actions.DeleteNode(node.id),
iD.actions.AddWayNode(way.id, datum.id));
iD.actions.AddWayNode(way.id, datum.id),
'added to an area');
controller.enter(iD.modes.DrawArea(wayId));
} else {
history.replace(
iD.actions.Noop(),
'added to an area');
controller.enter(iD.modes.DrawArea(wayId));
}
});

View File

@@ -34,7 +34,8 @@ iD.modes.DrawRoad = function(wayId, direction) {
// connect the way in a loop
history.replace(
iD.actions.DeleteNode(node.id),
iD.actions.AddWayNode(wayId, tailId, index));
iD.actions.AddWayNode(wayId, tailId, index),
'added to a road');
controller.enter(iD.modes.Select(way));
@@ -42,7 +43,8 @@ iD.modes.DrawRoad = function(wayId, direction) {
// connect the way to an existing node
history.replace(
iD.actions.DeleteNode(node.id),
iD.actions.AddWayNode(wayId, datum.id, index));
iD.actions.AddWayNode(wayId, datum.id, index),
'added to a road');
controller.enter(iD.modes.DrawRoad(wayId, direction));
@@ -51,11 +53,16 @@ iD.modes.DrawRoad = function(wayId, direction) {
var connectedIndex = iD.modes.chooseIndex(datum, d3.mouse(map.surface.node()), map);
history.replace(
iD.actions.AddWayNode(datum.id, node.id, connectedIndex));
iD.actions.AddWayNode(datum.id, node.id, connectedIndex),
'added to a road');
controller.enter(iD.modes.DrawRoad(wayId, direction));
} else {
history.replace(
iD.actions.Noop(),
'added to a road');
controller.enter(iD.modes.DrawRoad(wayId, direction));
}
});

View File

@@ -15,8 +15,7 @@ iD.modes.Select = function (entity) {
d3.event.sourceEvent.stopPropagation();
if (!dragging) {
dragging = iD.util.trueObj([entity.id].concat(
_.pluck(mode.history.graph().parentWays(entity.id), 'id')));
dragging = true;
mode.history.perform(iD.actions.Noop());
}
@@ -37,10 +36,14 @@ iD.modes.Select = function (entity) {
function remove() {
switch (entity.type) {
case 'way':
mode.history.perform(iD.actions.DeleteWay(entity.id));
mode.history.perform(
iD.actions.DeleteWay(entity.id),
'deleted a way');
break;
case 'node':
mode.history.perform(iD.actions.DeleteNode(entity.id));
mode.history.perform(
iD.actions.DeleteNode(entity.id),
'deleted a node');
}
mode.controller.exit();
@@ -59,11 +62,18 @@ iD.modes.Select = function (entity) {
.call(inspector);
inspector.on('changeTags', function(d, tags) {
mode.history.perform(iD.actions.ChangeEntityTags(d.id, tags));
mode.history.perform(
iD.actions.ChangeEntityTags(d.id, tags),
'changed tags');
}).on('changeWayDirection', function(d) {
mode.history.perform(iD.actions.ReverseWay(d.id));
mode.history.perform(
iD.actions.ReverseWay(d.id),
'reversed a way');
}).on('remove', function() {
remove();
}).on('close', function() {
mode.controller.exit();
});

View File

@@ -11,11 +11,6 @@ describe('iD.Graph', function() {
expect(graph.entity(entity.id)).to.equal(entity);
});
it('can be constructed with an annotation', function() {
var graph = iD.Graph({}, 'first graph');
expect(graph.annotation).to.equal('first graph');
});
if (iD.debug) {
it("is frozen", function () {
expect(Object.isFrozen(iD.Graph())).to.be.true;

View File

@@ -1,7 +1,6 @@
describe("History", function () {
describe("iD.History", function () {
var history, spy,
graph = iD.Graph([], "action"),
action = function() { return graph; };
action = function() { return iD.Graph(); };
beforeEach(function () {
history = iD.History();
@@ -16,13 +15,14 @@ describe("History", function () {
describe("#perform", function () {
it("updates the graph", function () {
history.perform(action);
var graph = iD.Graph();
history.perform(d3.functor(graph));
expect(history.graph()).to.equal(graph);
});
it("pushes the undo stack", function () {
history.perform(action);
expect(history.undoAnnotation()).to.equal("action");
it("pushes an undo annotation", function () {
history.perform(action, "annotation");
expect(history.undoAnnotation()).to.equal("annotation");
});
it("emits a change event", function () {
@@ -31,32 +31,27 @@ describe("History", function () {
expect(spy).to.have.been.called;
});
it("does not emit a change event when performing a noop", function () {
history.on('change', spy);
history.perform(iD.actions.Noop);
expect(spy).not.to.have.been.called;
});
it("performs multiple actions", function () {
var action1 = sinon.stub().returns(graph),
action2 = sinon.stub().returns(graph);
history.perform(action1, action2);
var action1 = sinon.stub().returns(iD.Graph()),
action2 = sinon.stub().returns(iD.Graph());
history.perform(action1, action2, "annotation");
expect(action1).to.have.been.called;
expect(action2).to.have.been.called;
expect(history.undoAnnotation()).to.equal("annotation");
});
});
describe("#replace", function () {
it("updates the graph", function () {
history.replace(action);
var graph = iD.Graph();
history.replace(d3.functor(graph));
expect(history.graph()).to.equal(graph);
});
it("replaces the undo stack", function () {
history.perform(action);
history.replace(action);
history.undo();
expect(history.undoAnnotation()).to.be.undefined;
it("replaces the undo annotation", function () {
history.perform(action, "annotation1");
history.replace(action, "annotation2");
expect(history.undoAnnotation()).to.equal("annotation2");
});
it("emits a change event", function () {
@@ -65,32 +60,27 @@ describe("History", function () {
expect(spy).to.have.been.called;
});
it("does not emit a change event when performing a noop", function () {
history.on('change', spy);
history.replace(iD.actions.Noop);
expect(spy).not.to.have.been.called;
});
it("performs multiple actions", function () {
var action1 = sinon.stub().returns(graph),
action2 = sinon.stub().returns(graph);
history.replace(action1, action2);
var action1 = sinon.stub().returns(iD.Graph()),
action2 = sinon.stub().returns(iD.Graph());
history.replace(action1, action2, "annotation");
expect(action1).to.have.been.called;
expect(action2).to.have.been.called;
expect(history.undoAnnotation()).to.equal("annotation");
});
});
describe("#undo", function () {
it("pops the undo stack", function () {
history.perform(action);
history.perform(action, "annotation");
history.undo();
expect(history.undoAnnotation()).to.be.undefined;
});
it("pushes the redo stack", function () {
history.perform(action);
history.perform(action, "annotation");
history.undo();
expect(history.redoAnnotation()).to.equal("action");
expect(history.redoAnnotation()).to.equal("annotation");
});
it("emits a change event", function () {
@@ -113,8 +103,8 @@ describe("History", function () {
describe("#reset", function () {
it("clears the version stack", function () {
history.perform(action);
history.perform(action);
history.perform(action, "annotation");
history.perform(action, "annotation");
history.undo();
history.reset();
expect(history.undoAnnotation()).to.be.undefined;