#18801 closed enhancement (fixed)
[PATCH RFC] Allow layers to determine autosave functionality
| Reported by: | taylor.smock | Owned by: | team |
|---|---|---|---|
| Priority: | normal | Milestone: | 20.06 |
| Component: | Core | Version: | |
| Keywords: | autosave | Cc: |
Description (last modified by )
The patch currently:
1) Modifies ImageData/NoteData to implement Data, but they return an empty collection of DataSources
2) Modifies AbstractModifiableLayer to have a function for autosave (takes a file argument), return false by default (to indicate not saved and not savable). It also adds an abstract method to get the data from the layer. The only addon I found to be affected by this was OpenQA, which can be modified by me.
3) AutosaveTask now just has a single Set<Data> instead of one each for DataSet/NodeData.
4) There is now a global instance of AutosaveTask, once it has been initialized. I should probably change all calls to be to getInstance.
5) It iterates through the layers, using the same if (changedData.remove) syntax, and then calls the layer's autosave method.
6) AutosaveTask also has a new method to indicate that data has been updated
7) Affected layers now implement getData and autosave, where the file was previously autosaved
EDIT: Mapillary is also affected
Attachments (2)
Change History (14)
by , 6 years ago
| Attachment: | 18801.patch added |
|---|
comment:1 by , 6 years ago
| Milestone: | → 20.03 |
|---|
comment:2 by , 6 years ago
| Description: | modified (diff) |
|---|
comment:3 by , 6 years ago
Replying to taylor.smock:
4) There is now a global instance of AutosaveTask, once it has been initialized. I should probably change all calls to be to
getInstance.
Why? I don't see the need.
comment:4 by , 6 years ago
| Description: | modified (diff) |
|---|
I haven't done that yet (in fact, I don't remember why I was thinking I might have to do it). (I'm assuming you are talking about the getInstance I was referring to).
comment:6 by , 6 years ago
| Milestone: | 20.03 → 20.04 |
|---|
comment:7 by , 6 years ago
| Description: | modified (diff) |
|---|
comment:9 by , 5 years ago
| Milestone: | 20.05 → 20.06 |
|---|
follow-up: 12 comment:10 by , 5 years ago
I'll modify you patch to make org.openstreetmap.josm.gui.layer.AbstractModifiableLayer#getData non-abstract and return null. Otherwise the following plugins would need to be adapted:
- https://trac.openstreetmap.org/browser/subversion/applications/editors/josm/plugins/MicrosoftStreetside/src/org/openstreetmap/josm/plugins/streetside/StreetsideLayer.java
- https://gitlab.com/gokaart/JOSM_OpenQA/blob/master/src/main/java/com/kaart/openqa/ErrorLayer.java
- https://gitlab.com/JOSM/plugin/Mapillary/blob/master/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/layer/MapillaryLayer.java
comment:12 by , 5 years ago
Replying to simon04:
I'll modify you patch to make
org.openstreetmap.josm.gui.layer.AbstractModifiableLayer#getDatanon-abstract and return null. Otherwise the following plugins would need to be adapted:
- https://trac.openstreetmap.org/browser/subversion/applications/editors/josm/plugins/MicrosoftStreetside/src/org/openstreetmap/josm/plugins/streetside/StreetsideLayer.java
- https://gitlab.com/gokaart/JOSM_OpenQA/blob/master/src/main/java/com/kaart/openqa/ErrorLayer.java
- https://gitlab.com/JOSM/plugin/Mapillary/blob/master/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/layer/MapillaryLayer.java
Fair enough (I could have modified two of the plugins on that list to work with this patch, but MS Streetside would still be a problem).



Initial patch (pre-existing tests pass)