Opened 14 years ago
Closed 13 years ago
#6233 closed enhancement (fixed)
[PATCH] Updated elements not sorted in upload dialog (Add/Delete are fine)
Reported by: | 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)
Change History (14)
by , 14 years ago
Attachment: | JOSM_upload.png added |
---|
by , 13 years ago
Attachment: | APIDataSet_sorting.patch added |
---|
comment:1 by , 13 years ago
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 by , 13 years ago
Summary: | [PATCH] Suggestion for the upload window → [PATCH] Updated elements not sorted in upload dialog (Add/Delete are fine) |
---|
comment:3 by , 13 years ago
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 by , 13 years ago
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.
comment:5 by , 13 years ago
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 by , 13 years ago
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 by , 13 years ago
Owner: | changed from | to
---|
comment:8 by , 13 years ago
I'm ready with the followup patch (cleanup but no functional changes) when you are.
comment:10 by , 13 years ago
@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.
by , 13 years ago
Attachment: | APIDataSet_cleanup.diff added |
---|
Cleanup of existing redundancy (per comment above). No functional changes.
Patch to sort Update items, just like Add and Delete items.