Modify

Opened 10 years ago

Closed 10 years ago

#6233 closed enhancement (fixed)

[PATCH] Updated elements not sorted in upload dialog (Add/Delete are fine)

Reported by: daswaldhorn@… Owned by: stoecker
Priority: normal Milestone:
Component: Core Version:
Keywords: patched Cc:

Description

In the upload window there are three parts for "objects to add", "objects to
modify" and "objects to delete". Is it possible to sort the entries inside
this parts according node, way and relation? At the moment there is a wild
mix, as you can see in the attachement (at least in the modify part).

Attachments (3)

JOSM_upload.png (5.4 KB) - added by anonymous 10 years ago.
APIDataSet_sorting.patch (1.6 KB) - added by brycenesbitt 10 years ago.
Patch to sort Update items, just like Add and Delete items.
APIDataSet_cleanup.diff (6.0 KB) - added by brycenesbitt 10 years ago.
Cleanup of existing redundancy (per comment above). No functional changes.

Download all attachments as: .zip

Change History (14)

Changed 10 years ago by anonymous

Attachment: JOSM_upload.png added

Changed 10 years ago by brycenesbitt

Attachment: APIDataSet_sorting.patch added

Patch to sort Update items, just like Add and Delete items.

comment:1 Changed 10 years ago by brycenesbitt

Keywords: patched added
Summary: Suggestion for the upload window[PATCH] Suggestion for the upload window

I've added a patch for this issue. Turns out Add and Delete items were sorted, but Update items not.

comment:2 Changed 10 years ago by brycenesbitt

Summary: [PATCH] Suggestion for the upload window[PATCH] Updated elements not sorted in upload dialog (Add/Delete are fine)

comment:3 Changed 10 years ago by stoecker

Instead of having 3 functions doing a sort with essentially the same code, can you make one function returning the Comparator and 3 simple calls for the sort instead?

comment:4 Changed 10 years ago by brycenesbitt

stoecker: I saw that, and thought about the same thing. In the end I decided to "go with the flow", and match the style of the existing code. This felt better for a first patch submission.

Note that the Comparators are not actually identical. Each of the three sorts slightly differently. So the end result would not be dramatically improved.

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

comment:5 Changed 10 years ago by stoecker

Sorry, but I don't see a difference in the comparator function part.

Anyway what about extracting the OsmPrimitiveComparator from trunk/src/org/openstreetmap/josm/gui/dialogs/SelectionListDialog.java#L919 into an own class and using this instead?

Having lots of different display rules is not a good idea at all.

You're right, that it is a good idea not to make to many changes first initial patches, but improving really bad code is also a good aim. ;-)

comment:6 Changed 10 years ago by brycenesbitt

stoecker: Ok, how about if you apply this patch as the first step, then I'll submit a second cleanup-only (no functional change) patch?

The difference in the Comparators is subtle, coming down to the sign of the return values. For some reason I don't understand Add and Update are deliberately sorted in a different order.

comment:7 Changed 10 years ago by brycenesbitt

Owner: changed from team to stoecker

comment:8 Changed 10 years ago by brycenesbitt

I'm ready with the followup patch (cleanup but no functional changes) when you are.

comment:9 Changed 10 years ago by bastiK

In [4104/josm]:

see #6233 (patch by brycenesbitt) - Updated elements not sorted in upload dialog (Add/Delete are fine)

comment:10 Changed 10 years ago by bastiK

@brycenesbitt: Probably simpler if you just submit the final patch. Please make patch from the root folder (not src). Anyway, doesn't matter that much, thanks for the fix.

Changed 10 years ago by brycenesbitt

Attachment: APIDataSet_cleanup.diff added

Cleanup of existing redundancy (per comment above). No functional changes.

comment:11 Changed 10 years ago by stoecker

Resolution: fixed
Status: newclosed

In [4113/josm]:

fix #6233 - better sorting for upload dialog

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain stoecker.
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.