Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#18381 closed enhancement (fixed)

[PATCH] Don't require a restart when a Tagging Preset is added/removed

Reported by: taylor.smock Owned by: Don-vip
Priority: normal Milestone: 19.12
Component: Core Version:
Keywords: restart, preset Cc:

Description

It would be nice to not have to restart after adding a preset.

Attachments (5)

18381.patch (3.4 KB ) - added by taylor.smock 5 years ago.
Add logic to destroy the preset menu and to rebuild it when the preset sources change
18381.1.patch (1.5 KB ) - added by taylor.smock 5 years ago.
Implement TaggingPresetListener in TagChecker and register itself as a listener with TaggingPresets
18381.2.patch (4.5 KB ) - added by taylor.smock 5 years ago.
Modify JOSM classes to implement a TaggingPresetListener, where appropriate
18381.3.patch (4.7 KB ) - added by taylor.smock 5 years ago.
Clear lists where appropriate
18381.tageditor.patch (1.7 KB ) - added by taylor.smock 5 years ago.
Patch for tageditor

Download all attachments as: .zip

Change History (21)

by taylor.smock, 5 years ago

Attachment: 18381.patch added

Add logic to destroy the preset menu and to rebuild it when the preset sources change

comment:1 by Don-vip, 5 years ago

Milestone: 19.12
Owner: changed from team to Don-vip
Status: newassigned

comment:2 by Don-vip, 5 years ago

Resolution: fixed
Status: assignedclosed

In 15582/josm:

fix #18381 - Don't require a restart when a Tagging Preset is added/removed (patch by taylor.smock)

comment:3 by GerdP, 5 years ago

Resolution: fixed
Status: closedreopened

This will not work with TagChecker as it builds internal data structures based on the TaggingPresets, but only when method initialize()is called.

Version 0, edited 5 years ago by GerdP (next)

by taylor.smock, 5 years ago

Attachment: 18381.1.patch added

Implement TaggingPresetListener in TagChecker and register itself as a listener with TaggingPresets

comment:4 by GerdP, 5 years ago

@taylor.smock: Did you check all the other sources which use e.g. TaggingPresets.getTaggingPresets()?
Sorry, I should have asked this before...

comment:5 by taylor.smock, 5 years ago

JOSM core:
src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetNameTemplateList.java: implements TaggingPresetListener already
src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetSelector.java: 18381.2.patch (only constructed once, and then stored)
src/org/openstreetmap/josm/gui/tagging/presets/items/PresetLink.java: It looks like a transient check to me (i.e., does a preset currently exist?)
src/org/openstreetmap/josm/data/osm/search/SearchCompiler.java: I don't think this needs anything
src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java: 18381.2.patch
src/org/openstreetmap/josm/data/validation/tests/TagChecker.java: 18381.1.patch

Plugins:
surveyor/src/org/openstreetmap/josm/plugins/surveyor/action/TaggingPresetAction.java: Doesn't look like an issue (not cached)
tageditor/src/org/openstreetmap/josm/plugins/tageditor/preset/ui/TabularPresetSelector.java: Needs work

I made the incorrect assumption that anything that needed to listen to changes to TaggingPresets already implemented a listener.

Last edited 5 years ago by taylor.smock (previous) (diff)

by taylor.smock, 5 years ago

Attachment: 18381.2.patch added

Modify JOSM classes to implement a TaggingPresetListener, where appropriate

by taylor.smock, 5 years ago

Attachment: 18381.3.patch added

Clear lists where appropriate

by taylor.smock, 5 years ago

Attachment: 18381.tageditor.patch added

Patch for tageditor

in reply to:  4 comment:6 by taylor.smock, 5 years ago

Replying to GerdP:

@taylor.smock: Did you check all the other sources which use e.g. TaggingPresets.getTaggingPresets()?
Sorry, I should have asked this before...

I think I got everything that is in the JOSM repository.

comment:7 by GerdP, 5 years ago

I agree reg. core, not sure about plugin surveyor. It seems to store a preset in field preset?

comment:8 by GerdP, 5 years ago

In 15583/josm:

see #18381: implement TaggingPresetListener to react on changes in tagging presets
regression from r15582

comment:9 by taylor.smock, 5 years ago

It does store the preset in a field, but it seemed to me that it was more of an initialization check. If I were to modify that class, I'd probably just check if the reset still exists.

That being said, does anyone still use the plugin? I installed it and then went to GPS -> surveyor, and it promptly crashed on me when I tried to use it (see #18412). I fixed that issue, but another one popped up.

comment:10 by GerdP, 5 years ago

I've never used surveyor, but it appears in some recent error reports, so it is installed by some users. Whatever that means.

comment:11 by Don-vip, 5 years ago

is this ticket fixed?

comment:12 by taylor.smock, 5 years ago

I believe it is fixed for core.

I need to make some changes to the surveyor plugin (as in, actually get it working), and then check and see if it needs to have a TaggingPresetListener. I'll file a separate bug for that, since it seems like it is going be more work.

I should probably make a separate bug for tageditor (see 18381.tageditor.patch, which I don't think has been applied yet).

comment:13 by GerdP, 5 years ago

Ah, sorry, forgot about 18381.tageditor.patch. I think in destroy() a super.destroy() is missing?

comment:14 by GerdP, 5 years ago

No, got it wrong. 18381.tageditor.patch applied with [o35258:35259]

comment:15 by Don-vip, 5 years ago

Resolution: fixed
Status: reopenedclosed

comment:16 by Don-vip, 5 years ago

In 15958/josm:

fix #18806 - see #18381 - fix tagging presets menu initialization (patch by taylor.smock)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.