[Patch] JosmAction.checkAndConfirmOutlyingOperation() should also handle dataset without download area

[GerdP]

We have some actions (unglue, delete, join area) which call this method to ask for user confirmation with texts like

"You are about to delete nodes outside of the area you have downloaded."

This message doesn't cover the case that the user didn't donwload any area when e.g. an overpass query was used or maybe download object.
I think the text should be changed to

"You are about to delete nodes not inside any area you have downloaded."

and the test should be changed to detect the corresponding case.

[GerdP]

The dialog should probably be used by a lot more actions. In a later step I want to replace it by a dialog which asks the user if downloading more data is OK.

If you don't like the new text please suggest an alternative.

[skyper]

How about, You are about to delete nodes outside of an area you have completely downloaded. ?

[GerdP]

Replying to skyper:

How about, You are about to delete nodes outside of an area you have completely downloaded. ?

I think this still implies that there is an area which was downloaded.
Maybe You are about to delete nodes which can have other referrers not yet downloaded.?

[skyper]

Yes, better and in sync with possible future warning You are about to move nodes which can have other referrers not yet downloaded.

[GerdP]

In 16365/josm:

fix #19121: JosmAction.checkAndConfirmOutlyingOperation() should also handle dataset without download area

• adjust message texts
• also check if object is old and no download area exists

[skyper]

I am not sure about unglue and ways with id:0:

1. split a way with at least one node inside downloaded area
2. unglue node of just created new way

Think here a warning is still useful.

If you "unglue" or "extract node" in advance, similar can happen with nodes with id:0.

[GerdP]

Not related to this ticket. I did not suppress any warning. I've only added one for the case that no download area exists.

[skyper]

Replying to GerdP:

I've only added one for the case that no download area exists.

Exactly, I might be reading this wrong but, in my eyes, we'll miss some cases with ways with id:0 :

trunk/src/org/openstreetmap/josm/command/Command.java

@@ -229 +229 @@
                 res |= IS_INCOMPLETE;
             } else if (osm.isOutsideDownloadArea()
-                    && (ignore == null || !ignore.contains(osm))) {
+                    || (!osm.isNew() && osm.getDataSet() != null && osm.getDataSet().getDataSourceBounds().isEmpty())
+                            && (ignore == null || !ignore.contains(osm))) {
                 res |= IS_OUTSIDE;
             }

[GerdP]

!osm.isNew() is equivalent to id:0. I don't like to use osm.getId() == 0 because one always has to check twice to make sure that osm.getId() always returns 0 for negative ids.

[skyper]

Thanks for explanation, guess I have to train my eyes/brain to better read/understand diffs.

[GerdP]

In fact I should have written osm.isNew() is equivalent to osm.getId() == 0. For a new object we cannot download parents.

Looking at the patch again I think a pair of() is missing :(

[GerdP]

In 16369/josm:

see #19121: JosmAction.checkAndConfirmOutlyingOperation() should also handle dataset without download area

• add missing brackets so that possible ignore list works

[GerdP]

In 16371/josm:

see #19121: fix unit test

[Klumbumbus]

Milestone renamed

[GerdP]

r16365 introduced a regression. When an empty relation is deleted and there is no download area a popup "You are about to delete nodes which can have other referrers not yet downloaded..." is shown.

[GerdP]

In 16513/josm:

fix #19121: JosmAction.checkAndConfirmOutlyingOperation() should also handle dataset without download area

• restrict additional check if dataset has a download area to nodes