Skip to content

Commit 6b82c58

Browse files
tyrasdtyrasd
authored
Merge pull request #10737 from burrscurr/way-reverse-dont-change-red-turn-direction
2 parents fd96190 + 8b5c9ba commit 6b82c58

File tree

3 files changed

+97
-26
lines changed

3 files changed

+97
-26
lines changed

‎CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
4545
* Render `*_link` roads narrower than their "regular" counterpart ([#10722])
4646
#### :scissors: Operations
4747
* Fix splitting of closed ways (or areas) when two or more split-points are selected
48+
* Keep `red_turn:left/right` tags unchanged when reversing a way ([#10737], thanks [@burrscurr])
4849
#### :camera: Street-Level
4950
#### :white_check_mark: Validation
5051
* Add warning if aeroways cross each other, buildings or highways ([#9315], thanks [@k-yle])
@@ -71,6 +72,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
7172
[#10720]: https://github.com/openstreetmap/iD/issues/10720
7273
[#10722]: https://github.com/openstreetmap/iD/pull/10722
7374
[#10727]: https://github.com/openstreetmap/iD/issues/10727
75+
[#10737]: https://github.com/openstreetmap/iD/pull/10737
7476
[#10747]: https://github.com/openstreetmap/iD/issues/10747
7577
[#10748]: https://github.com/openstreetmap/iD/issues/10748
7678
[#10755]: https://github.com/openstreetmap/iD/issues/10755
@@ -80,6 +82,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
8082
[@hlfan]: https://github.com/hlfan
8183
[@Deeptanshu-sankhwar]: https://github.com/Deeptanshu-sankhwar
8284
[@draunger]: https://github.com/draunger
85+
[@burrscurr]: https://github.com/burrscurr
8386

8487

8588
# 2.31.1

‎modules/actions/reverse.js

+34-26
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,8 @@ References:
1818
http://wiki.openstreetmap.org/wiki/Key:traffic_sign#On_a_way_or_area
1919
*/
2020
export function actionReverse(entityID, options) {
21-
var ignoreKey = /^.*(_|:)?(description|name|note|website|ref|source|comment|watch|attribution)(_|:)?/;
2221
var numeric = /^([+\-]?)(?=[\d.])/;
2322
var directionKey = /direction$/;
24-
var turn_lanes = /^turn:lanes:?/;
2523
var keyReplacements = [
2624
[/:right$/, ':left'],
2725
[/:left$/, ':right'],
@@ -42,13 +40,27 @@ export function actionReverse(entityID, options) {
4240
forwards: 'backward',
4341
backwards: 'forward',
4442
};
45-
// tags whose values should not be reversed when certain other tags are also present
46-
// https://github.com/openstreetmap/iD/issues/10128
47-
const valueReplacementsExceptions = {
48-
'side': [
49-
{highway: 'cyclist_waiting_aid'}
50-
]
51-
};
43+
// For some tags, keys or values like left/right/… don't refer to
44+
// way direction and thus should not be reversed.
45+
const keysToKeepUnchanged = [
46+
// https://github.com/openstreetmap/iD/issues/10736
47+
/^red_turn:(right|left):?/
48+
];
49+
// If a key matches the key regex and any of the provided context
50+
// tag sets, it will not be reversed.
51+
const keyValuesToKeepUnchanged = [{
52+
keyRegex: /^.*(_|:)?(description|name|note|website|ref|source|comment|watch|attribution)(_|:)?/,
53+
prerequisiteTags: [{}]
54+
}, {
55+
// Turn lanes are left/right to key (not way) direction - #5674
56+
keyRegex: /^turn:lanes:?/,
57+
prerequisiteTags: [{}]
58+
}, {
59+
// https://github.com/openstreetmap/iD/issues/10128
60+
keyRegex: /^side$/,
61+
prerequisiteTags: [{highway: 'cyclist_waiting_aid'}]
62+
}
63+
];
5264
var roleReplacements = {
5365
forward: 'backward',
5466
backward: 'forward',
@@ -82,6 +94,9 @@ export function actionReverse(entityID, options) {
8294

8395

8496
function reverseKey(key) {
97+
if (keysToKeepUnchanged.some(keyRegex => keyRegex.test(key))) {
98+
return key;
99+
}
85100
for (var i = 0; i < keyReplacements.length; ++i) {
86101
var replacement = keyReplacements[i];
87102
if (replacement[0].test(key)) {
@@ -93,13 +108,17 @@ export function actionReverse(entityID, options) {
93108

94109

95110
function reverseValue(key, value, includeAbsolute, allTags) {
96-
if (ignoreKey.test(key)) return value;
97-
98-
// Turn lanes are left/right to key (not way) direction - #5674
99-
if (turn_lanes.test(key)) {
100-
return value;
111+
for (let { keyRegex, prerequisiteTags } of keyValuesToKeepUnchanged) {
112+
if (keyRegex.test(key) && prerequisiteTags.some(expectedTags =>
113+
Object.entries(expectedTags).every(([k, v]) => {
114+
return allTags[k] && (v === '*' || allTags[k] === v);
115+
})
116+
)) {
117+
return value;
118+
}
119+
}
101120

102-
} else if (key === 'incline' && numeric.test(value)) {
121+
if (key === 'incline' && numeric.test(value)) {
103122
return value.replace(numeric, function(_, sign) { return sign === '-' ? '' : '-'; });
104123

105124
} else if (options && options.reverseOneway && key === 'oneway') {
@@ -122,17 +141,6 @@ export function actionReverse(entityID, options) {
122141
}
123142
}).join(';');
124143
}
125-
126-
if (valueReplacementsExceptions[key] && valueReplacementsExceptions[key].some(exceptionTags =>
127-
Object.keys(exceptionTags).every(k => {
128-
const v = exceptionTags[k];
129-
return allTags[k] && (v === '*' || allTags[k] === v);
130-
})
131-
)) {
132-
// don't reverse, for example, side=left/right on highway=cyclist_waiting_aid features
133-
// see https://github.com/openstreetmap/iD/issues/10128
134-
return value;
135-
}
136144
return valueReplacements[value] || value;
137145
}
138146

‎test/spec/actions/reverse.js

+60
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ describe('iD.actionReverse', function () {
1515
expect(graph.entity(way.id).tags).to.eql({'highway': 'residential'});
1616
});
1717

18+
1819
describe('reverses directional tags on nodes', function () {
1920
it('reverses relative directions', function () {
2021
var node1 = iD.osmNode({ tags: { 'direction': 'forward' } });
@@ -95,6 +96,7 @@ describe('iD.actionReverse', function () {
9596
});
9697
});
9798

99+
98100
describe('reverses oneway', function () {
99101
it('preserves oneway tags', function () {
100102
var way = iD.osmWay({tags: {'oneway': 'yes'}});
@@ -588,6 +590,47 @@ describe('iD.actionReverse', function () {
588590
var target = graph.entity(node2.id);
589591
expect(target.tags['traffic_signals:direction']).to.eql('empty');
590592
});
593+
});
594+
595+
596+
describe('does not reverse values which are relative to another reversed tag', function () {
597+
it('preserves the turn direction of a single lane road', function () {
598+
var way = iD.osmWay({tags: {'turn:lanes': 'right'}});
599+
var graph = iD.actionReverse(way.id)(iD.coreGraph([way]));
600+
var target = graph.entity(way.id);
601+
expect(target.tags['turn:lanes']).to.eql('right');
602+
});
603+
604+
it('preserves the turn directions of a multi-lane road', function () {
605+
var way = iD.osmWay({tags: {'turn:lanes': 'through|through|right'}});
606+
var graph = iD.actionReverse(way.id)(iD.coreGraph([way]));
607+
var target = graph.entity(way.id);
608+
expect(target.tags['turn:lanes']).to.eql('through|through|right');
609+
});
610+
611+
// https://github.com/openstreetmap/iD/issues/5674
612+
it('preserves the turn direction of each direction with a single lane', function () {
613+
var way = iD.osmWay({tags: {'turn:lanes:forward': 'right', 'turn:lanes:backward': 'left'}});
614+
var graph = iD.actionReverse(way.id)(iD.coreGraph([way]));
615+
var target = graph.entity(way.id);
616+
expect(target.tags['turn:lanes:backward']).to.eql('right');
617+
expect(target.tags['turn:lanes:forward']).to.eql('left');
618+
});
619+
620+
it('preserves the turn direction of each direction with multiple lanes', function () {
621+
var way = iD.osmWay({tags: {'turn:lanes:forward': 'through|right', 'turn:lanes:backward': 'through|through|left'}});
622+
var graph = iD.actionReverse(way.id)(iD.coreGraph([way]));
623+
var target = graph.entity(way.id);
624+
expect(target.tags['turn:lanes:backward']).to.eql('through|right');
625+
expect(target.tags['turn:lanes:forward']).to.eql('through|through|left');
626+
});
627+
628+
it('preserves the turn direction of explicitly bidirectional turn lane values', function () {
629+
var way = iD.osmWay({tags: {'turn:lanes:both_ways': 'left'}});
630+
var graph = iD.actionReverse(way.id)(iD.coreGraph([way]));
631+
var target = graph.entity(way.id);
632+
expect(target.tags['turn:lanes:both_ways']).to.eql('left');
633+
});
591634

592635
it('preserves the value of the side tag of a cycling waiting aid', function () {
593636
var node1 = iD.osmNode();
@@ -603,5 +646,22 @@ describe('iD.actionReverse', function () {
603646
expect(target.tags.direction).to.eql('backward');
604647
expect(target.tags.side).to.eql('right');
605648
});
649+
650+
it('preserves the direction of a red turn at a traffic signal', function () {
651+
var node1 = iD.osmNode();
652+
var node2 = iD.osmNode({tags: {
653+
'highway': 'traffic_signals',
654+
'traffic_signals:direction': 'forward',
655+
'red_turn:right': 'no',
656+
'red_turn:right:bicycle': 'yes'
657+
}});
658+
var node3 = iD.osmNode();
659+
var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]});
660+
var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way]));
661+
var target = graph.entity(node2.id);
662+
expect(target.tags['traffic_signals:direction']).to.eql('backward');
663+
expect(target.tags['red_turn:right']).to.eql('no');
664+
expect(target.tags['red_turn:right:bicycle']).to.eql('yes');
665+
});
606666
});
607667
});

0 commit comments

Comments
 (0)