Modify

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#6250 closed enhancement (fixed)

[patch] Undo selection - shortcut needed (or access to history list from plugins)

Reported by: akks Owned by: team
Priority: major Milestone:
Component: Core Version: latest
Keywords: undo selection Cc: a.dezhin@…, bastiK

Description

Discussion on OSM Russia forum shows that there are people interested in Restore selection shortcut (including me).

We know about selection history drop-down list, but shortcut is also needed.

There is no access to this history for plugins (SelectionListDialog.SelectionListModel.history). Maintaining this history is rather CPU and memory- demanding, so we wanted to use the same object.
Can someone from core developers help us?

Some other plugins can reuse last selections and save resources too!

I can use Main.main.undoRedo to select newly added entities, but this is another action.
One more proposal - selecting entities from buffer.

Planned key for Undo selection - Shift-Z.

Attachments (6)

selectionHistory.patch (2.4 KB) - added by akks 10 years ago.
patch for core to expose selectionHistory
undoSelection_v2.patch (10.4 KB) - added by akks 10 years ago.
patch for core - adds getSelectionHistory to DataSet
undoSelection_v3.patch (3.1 KB) - added by akks 10 years ago.
undoSelection_v3.2.patch (3.1 KB) - added by akks 10 years ago.
runjosm.patch (429 bytes) - added by akks 10 years ago.
undoSelection_v4.patch (4.0 KB) - added by akks 10 years ago.
ready core patch

Download all attachments as: .zip

Change History (34)

comment:1 Changed 10 years ago by akks

Version: latest

comment:2 Changed 10 years ago by Aleksandr Dezhin

Cc: a.dezhin@… added

comment:3 Changed 10 years ago by stoecker

Patches welcome.

comment:4 Changed 10 years ago by akks

Errr.. Patch for core that uses static Main.main.selectionHistory as SelectionListModel.history and exposes it for all users?
Not too radical to accept?

Or just Shift-Z implementation to insert into core, tight linked with SelectionListDialog? (Do not know, how to implement correctly)

Last edited 10 years ago by akks (previous) (diff)

comment:5 Changed 10 years ago by stoecker

JOSM has a "expose interfaces when necessary" concept instead of a "designed API". So yes, designing plugin interfaces on-the-fly is a acceptable way to go.

Advantage: Easy to program and maintain, low developer requirements.
Disadvantage: Unstable API forcing plugin updates.

So please design a proper way to access the data (not simply making it public) and supply a patch for this.

comment:6 Changed 10 years ago by akks

Can I take Main.main.undoRedo as a good example? (something like singleton)

Or I could add it as getCurrentDataSet().getSelectionHistory(). Is it better? (not not know main conceptions of JOSM construction)

Of course I do not want to expose history of Swing model of certain dialog to plugins)))

Last edited 10 years ago by akks (previous) (diff)

Changed 10 years ago by akks

Attachment: selectionHistory.patch added

patch for core to expose selectionHistory

comment:7 Changed 10 years ago by akks

Please review this minimal patch to make save selection history available for all (and apply it to core in some form).
I have also found some strange behaviour - when entities are deleted, they remain present in selection history lists!
I tried to avoid this in Utilsplugin2 addition, but not from 'selection'-triangle-button-menu. It it a feature?

I am implementing cyclic selection of history lists by Shift-Z using patched core.

comment:8 Changed 10 years ago by stoecker

Summary: Undo selection - shortcut needed (or access to history list from plugins)[patch] Undo selection - shortcut needed (or access to history list from plugins)

comment:9 Changed 10 years ago by stoecker

Two issues:

  • Better provide an Interface like getSelectionHistory() and if required add...(). Public variables usually require redesigns later. There is no need to involve Main class.
  • Cleanup your patches, so nothing remains, which is not wanted (your changes to build.xml).

comment:10 Changed 10 years ago by akks

Different dialogs and actions are rather hard to link together.
Are there any good examples of interface usage in JOSM code? I saw only listeners, which are not suitable here, I think.
Projection proj, UndoRedoHandler undoRedo, Preferences pref, OsmValidator validator are still in Main.

Main question - getSelectionHistory() of what class? DataSet maybe?

I sent build.xml intentionally, as a missing simple way to run compiled josm (for NetBeans). Of course I can remove it.

Last edited 10 years ago by akks (previous) (diff)

Changed 10 years ago by akks

Attachment: undoSelection_v2.patch added

patch for core - adds getSelectionHistory to DataSet

comment:11 Changed 10 years ago by akks

Please look at new patch, where selectionHistory is moved to DataSet class and is taken from it in SelectionListModel.

build.xml excluded (please include runjosm.xml or something equivalent later - test is not enough)

Most of the changed lines are newly added @Override annotations (Netbeans advice).

UtilsPlugin2 with cyclic UndoSelection is ready and waiting for core changes to be commited (depends on newly added methods).

comment:12 Changed 10 years ago by stoecker

Cc: bastiK added

@bastiK:

Selection list in DataSet sounds good to me. Do you see any side effects?

comment:13 in reply to:  12 Changed 10 years ago by bastiK

Replying to stoecker:

@bastiK:

Selection list in DataSet sounds good to me. Do you see any side effects?

Sounds good.

  • Please provide patches without cosmetic changes, this includes indentation, @Override and the like. If you are using Eclipse, better turn off Save Actions while hacking JOSM.
  • the update has to go in editLayerChanged() and not in dataChanged()
  • clearSelectionHistory() should move to the DataSet

comment:14 Changed 10 years ago by akks

You can edit http://josm.openstreetmap.de/wiki/DevelopersGuide/StyleGuide to avoid patches with many-many @Overrides)

Searching other plugins for dependencies.. (moved all clearSelectionHistory methods to DataSet).
Almost ready.

comment:15 in reply to:  14 Changed 10 years ago by bastiK

Replying to akks:

You can edit http://josm.openstreetmap.de/wiki/DevelopersGuide/StyleGuide to avoid patches with many-many @Overrides)

True, this is from Java 5 times, where interface implementations did not have this annotation.

Searching other plugins for dependencies.. (moved all clearSelectionHistory methods to DataSet).
Almost ready.

Another thing: You should test, what happens if you close selection list dialog, restart JOSM and never open this dialog. I suspect, that selection history will not work then. DataSet has to "take over" completely and the dialog would merely read this list.

comment:16 Changed 10 years ago by akks

Here is the patch (and build.xml patch if you consider it - really convenient feature)

If dialog is not created, no selection history is available. But there are no exceptions)

Changed 10 years ago by akks

Attachment: undoSelection_v3.patch added

Changed 10 years ago by akks

Attachment: undoSelection_v3.2.patch added

Changed 10 years ago by akks

Attachment: runjosm.patch added

comment:17 Changed 10 years ago by akks

Now one can disable selection history and save processor resources.
If I put part of SelectionListModel closer to DataSet as separate listener, it will maintain history continuously.
This needs much bigger kernel change also and can cause many other potential bugs (messing with at least 4 listeners!)
(we cannot just move setJOSMSelection() and functions calling it to other class, because it also maintains 'selection' - copied collection and two listener methods call it)

So if we need better architecture, you can improve it yourself. I do not want to ruin half of Josm after spending hours)

Last edited 10 years ago by akks (previous) (diff)

comment:18 Changed 10 years ago by akks

NullPointerException - patching patch one more)

comment:19 Changed 10 years ago by akks

Patch is ready.

Changed 10 years ago by akks

Attachment: undoSelection_v4.patch added

ready core patch

comment:20 Changed 10 years ago by akks

Removed two debugging output lines. Does this patch have a chance? ))

comment:21 Changed 10 years ago by akks

Err.. Can you include current version of the patch?
If no, please give me some advice on implementing listeners (SelectionChangedListener, LayerChangedListener interfaces) in other classes then SelectionListDialog.SelectionListModel.

comment:22 Changed 10 years ago by bastiK

Resolution: fixed
Status: newclosed

In [4064/josm]:

applied #6250 (patch by akks) - move selection history to dataset

comment:23 Changed 10 years ago by anonymous

Thank you) This week I will add UndoSelection action to Utilsplugin2.

comment:24 Changed 10 years ago by bastiK

akks: Please take care when committing plugin code, see http://josm.openstreetmap.de/ticket/6201#comment:4 - the same applies for utilsplugin2.

In addition, we have to make sure, that users of "tested" JOSM don't get the new version of the plugin, because it would be incompatible. For this, the Mainversion in build.xml is set to 4064. But please make sure, that the last binary of utilsplugin2 has correct plugin version number in Manifest. If in doubt, just check in the sources and let us update the binary.

Cheers, bastiK

comment:25 Changed 10 years ago by akks

OK, I will change version and check it on old josm before committing. Jar and sources will be commited separately.

In fact, I have just noticed, that my last commit (select modified ways/nodes) does not have two source files at all - only jar and png-s (

comment:26 Changed 10 years ago by akks

check-in completed.

All of versions gets the correct version of plugin, except 3695 - it install old version and then asks to update If user agree, he gets newest version of the plugin (with MainVersion 4064 in manifest) and one no-working button.

3795, 4011 , 4064 work with their different plugin versions correctly (cleaned JOSM settings folder before running jar and installing Utilsplugin2)

comment:27 Changed 10 years ago by bastiK

Seems to be all right, even 3695 work for me - may be a temporary problem. You can add yourself to the list of authors, if you like.

comment:28 Changed 10 years ago by akks

Thank you)

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.