[Patch] reverter plugin

[geow]

Edits on nodes won't be reverted, see ​osmwww:changeset/82149018 and ​osmwww:changeset/82149070

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-02-26 10:50:27 +0100 (Wed, 26 Feb 2020)
Build-Date:2020-02-26 09:52:41
Revision:15937
Relative:URL: ^/trunk

Identification: JOSM/1.5 (15937 en) Mac OS X 10.13.6
OS Build number: Mac OS X 10.13.6 (17G11023)
Memory Usage: 1221 MB / 2731 MB (961 MB allocated, but free)
Java version: 1.8.0_241-b07, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: Display 69678912 1920x1080
Maximum Screen Size: 1920x1080
Dataset consistency test: No problems found

Plugins:
+ DirectUpload (35248)
+ FastDraw (35360)
+ ImproveWay (29)
+ InfoMode (35248)
+ apache-commons (35362)
+ buildings_tools (35248)
+ ejml (35122)
+ geotools (35169)
+ jaxb (35092)
+ jts (35122)
+ measurement (35248)
+ opendata (35330)
+ reverter (35313)
+ terracer (35327)
+ utilsplugin2 (35334)

Tagging presets:
+ https://josm.openstreetmap.de/josmfile?page=Presets/OneClick&zip=1
+ https://raw.githubusercontent.com/yopaseopor/traffic_signs_preset_JOSM/master/DE.zip
+ https://josm.openstreetmap.de/josmfile?page=Presets/Leaftype&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Mountainbike&zip=1

Last errors/warnings:
- E: Failed to locate image 'traffic_signs_presets/ES_tertiary.png'
- W:  Tertiary: Could not get presets icon traffic_signs_presets/ES_tertiary.png
- W: No revert commands found for changeset 82149018
- W: No revert commands found for changeset 82149018
- W: No revert commands found for changeset 82149018

[skyper]

proper interlinks no redirects

[GerdP]

Hmm, this is quite special. Both changesets removed tags with prefix "gpx:" and those are consideted "uninteresting" like e.g. created_by or source. These tags are ignored when two object versions are compared, thus nothing is reverted.
If I got that right the error is in JOSM core.
The javadoc for method OsmPrimitive.hasEqualSemanticAttributes(OsmPrimitive other) claims that it returns true if "both have the same tags" but it calls a method which ignores the "uninteresting" tags.

A possible solution for reverter would be to make method OsmPrimitive.hasEqualSemanticAttributes(OsmPrimitive other, boolean testInterestingTagsOnly) public and call this in reverter.

[GerdP]

Attached patch changes reverter to compare all tags. It doesn't require a change in core.
Drawback: This would also revert changes where so-called discardable tags were silently removed.
I just noticed that "created_by" is one of them, the full list is in AbstractPrimitive.

[GerdP]

ignore discardable tags only

[Don-vip]

You can simply call AbstractPrimitive.getInterestingTags()

[GerdP]

Replying to Don-vip:

You can simply call AbstractPrimitive.getInterestingTags()

I think that is what is happening now.

[Don-vip]

Replying to GerdP:

Replying to Don-vip:

You can simply call AbstractPrimitive.getInterestingTags()

I think that is what is happening now.

? I don't understand. You don't need to write all this code:

private static Map<String, String> getNonDiscardableTags(OsmPrimitive p) {
    Map<String, String> result = new HashMap<>();
    for (Map.Entry<String, String> e : p.getKeys().entrySet()) {
    if (!AbstractPrimitive.getDiscardableKeys().contains(e.getKey()))
        result.put(e.getKey(), e.getValue());
    }
    return result;
}

Simply call:

p.getInterestingTags()

[GerdP]

The result would not be the same. Look at the implementation of isUninterestingKey which is called in getInterestingTags

[skyper]

So make the call of isUninterestingKey optional.

[GerdP]

I see no need for an optiion. In addition to the discardable tags the following tag keys are considered "uninteresting":

"source", "source_ref", "source:", "comment","watch", "watch:", "description", "attribution", "note", "fixme", "FIXME"

In addition all tag keys starting with "gpx:" and all tags listed in preference tags.workinprogress
In my eyes those additional tags should not be ignored when a cs is reverted.

[GerdP]

@Vincent: Do you see a better solution?

[Don-vip]

@Gerd: I don't. Go ahead :)

[skyper]

Replying to Don-vip:

@Gerd: I don't. Go ahead :)

stoecker on #18673 comment 8:

JOSM does not always follow "global decision and the discussions". We simply do something (like replacing 3 variants of boolean with the yes/no standard we have now).

this should not be reverted.

[GerdP]

@skyper: What do you mean?

[skyper]

e.g. reverter should not revert if true/false was replaced with yes/no. I did not look into code but I am not sure if these value changes are ignored.

[GerdP]

I think a revert should revert everything. The discardable tags are already an exception which I find problematic. If we want to allow that the removement of gpx:ele can be reverted why not also revert changes between boolean values?

[skyper]

Ok, if you gonna use JOSM to upload, the discardable tags will be removed again, so you are right. Better revert everything or at least let the user decide with options.

[stoecker]

e.g. reverter should not revert if true/false was replaced with yes/no. I did not look into code but I am not sure if these value changes are ignored.

I don't think that is important, as true/false is mainly gone from the database for boolean values.

I think a revert should revert everything. The discardable tags are already an exception which I find problematic. If we want to allow that the removement of gpx:ele can be reverted why not also revert changes between boolean values?

Because JOSM silently will re-revert them when you do anything with the tags :-) A revert thus means you'll upload a NULL-change to the server. For boolean-value it probably needs any action with the tags for the change to trigger.

[GerdP]

@Dirk: So you also agree that 18929.2.patch is okay?

[stoecker]

Replying to GerdP:

@Dirk: So you also agree that 18929.2.patch is okay?

Probably. Sounds sane to me, but I didn't test it, so I cannot say if it IS also sane. But we'll see when applied :-)

[Don-vip]

Replying to GerdP:

I think a revert should revert everything.

The reverter plugin is incredibly complex. Reverting everything will allow to simplify it, so I agree with you.

[GerdP]

see [o35381:35383] (18929.2.patch)