Modify

Opened 19 months ago

Closed 16 months ago

Last modified 16 months ago

#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 taylor.smock)

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)

18801.patch (14.5 KB) - added by taylor.smock 19 months ago.
Initial patch (pre-existing tests pass)
18801.1.patch (13.7 KB) - added by taylor.smock 17 months ago.
Remove unnecessary global instance

Download all attachments as: .zip

Change History (14)

Changed 19 months ago by taylor.smock

Attachment: 18801.patch added

Initial patch (pre-existing tests pass)

comment:1 Changed 19 months ago by Don-vip

Milestone: 20.03

comment:2 Changed 19 months ago by taylor.smock

Description: modified (diff)

comment:3 in reply to:  description Changed 18 months ago by Don-vip

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 Changed 18 months ago by taylor.smock

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:5 Changed 18 months ago by Don-vip

I don't see the need to have a global instance of AutosaveTask.

comment:6 Changed 18 months ago by Don-vip

Milestone: 20.0320.04

Changed 17 months ago by taylor.smock

Attachment: 18801.1.patch added

Remove unnecessary global instance

comment:7 Changed 17 months ago by taylor.smock

Description: modified (diff)

comment:8 Changed 17 months ago by Klumbumbus

Milestone: 20.0420.05

Milestone renamed

comment:9 Changed 16 months ago by simon04

Milestone: 20.0520.06

comment:11 Changed 16 months ago by simon04

Resolution: fixed
Status: newclosed

In 16548/josm:

fix #18801 - Allow layers to determine autosave functionality (patch by taylor.smock, modified)

comment:12 in reply to:  10 Changed 16 months ago by taylor.smock

Replying to simon04:

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:

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).

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.