Modify

Opened 6 weeks ago

Closed 3 weeks ago

Last modified 11 days ago

#19161 closed enhancement (fixed)

History browser: relation member table lacks reversed diff incidator

Reported by: OttawaHiking Owned by: team
Priority: normal Milestone: 20.05
Component: Core Version: tested
Keywords: history; versions; members, comparison Cc: Don-vip

Description (last modified by skyper)

What steps will reproduce the problem?

  1. Download object... relation 148838.
  2. Select the relation (boundary[2] "United States") and click "History".
  3. Click on tab "Members".
  4. Compare versions 613 and 614 (the last two versions as of this writing).

What is the expected result?

JOSM is expects to highlight the differences between the two versions.

What happens instead?

JOSM shows incorrectly that the versions are very different, because version 613 is not shown properly.


Please provide any additional information below. Attach a screenshot if possible.

Version 613 (as it is on the server) could be seen by choosing to compare version 613 with itself, i.e., choosing version 613 in both column A and column B of the left panel.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-04-06 02:17:07 +0200 (Mon, 06 Apr 2020)
Build-Date:2020-04-06 00:18:43
Revision:16239
Relative:URL: ^/trunk

Identification: JOSM/1.5 (16239 en) Windows 10 64-Bit
OS Build number: Windows 10 Pro 1909 (18363)
Memory Usage: 859 MB / 1794 MB (609 MB allocated, but free)
Java version: 1.8.0_181-b13, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1920x1080
Maximum Screen Size: 1920x1080
Dataset consistency test: No problems found

Plugins:
+ reverter (35409)
+ undelete (35405)

Last errors/warnings:
- W: org.openstreetmap.josm.tools.bugreport.BugReportSender$BugReportSenderException: java.net.SocketTimeoutException: connect timed out. Cause: java.net.SocketTimeoutException: connect timed out

Attachments (5)

History for relation 148838.jpg (374.6 KB) - added by OttawaHiking 6 weeks ago.
version 613.jpg (852.5 KB) - added by OttawaHiking 6 weeks ago.
version 614.jpg (870.4 KB) - added by OttawaHiking 6 weeks ago.
snip.png (67.8 KB) - added by GerdP 3 weeks ago.
2020-05-17-220545_1118x658_scrot.png (65.7 KB) - added by simon04 3 weeks ago.

Change History (21)

Changed 6 weeks ago by OttawaHiking

comment:1 Changed 6 weeks ago by GerdP

At a first glance I'd say you changed the order of the members.

comment:2 Changed 6 weeks ago by GerdP

Resolution: worksforme
Status: newclosed

You even documented it in your changeset comment "Reordered boundary members". I suggest to revert the change for the very reason that it makes comparing the revisions difficult.

Changed 6 weeks ago by OttawaHiking

Attachment: version 613.jpg added

Changed 6 weeks ago by OttawaHiking

Attachment: version 614.jpg added

comment:3 Changed 6 weeks ago by OttawaHiking

Resolution: worksforme
Status: closedreopened

@GerdP: I uploaded images that show the initial members of versions 613 and 614. The first three members of both versions are the same, but the comparison is not catching this similarity. Is not it a problem?

Because I reordered segments of a very long boundary in a sequential way, without making any additional changes on the top, the resulting relation can serve as an excellent reference point for a number of following edits. OSM Inspector was complaining about the Contiguous United States boundary, so I opted to reorder its segments.


Last edited 3 weeks ago by simon04 (previous) (diff)

comment:4 Changed 5 weeks ago by OttawaHiking

Resolution: wontfix
Status: reopenedclosed

@GerdP: I guess you would argue that inversion of the sequence of members (regardless of their role in a relation) is giving a slightly better match in this case, so it was applied.

comment:5 in reply to:  4 Changed 5 weeks ago by GerdP

Resolution: wontfix
Status: closedreopened

Replying to OttawaHiking:

@GerdP: I guess you would argue that inversion of the sequence of members (regardless of their role in a relation) is giving a slightly better match in this case, so it was applied.

No, sorry, I got you wrong and was to lazy to double check :(
You are right I think what you see is an effect of this ticket: #6994
Still, the actual result for r148838 looks very different compared to picture shown in that ticket.

Last edited 5 weeks ago by GerdP (previous) (diff)

comment:6 Changed 5 weeks ago by GerdP

Cc: Don-vip added
Type: defectenhancement

OK, I've learned a lot of new things about this dialog. I see two possible improvements:

  • When way nodes are compared, there is a small arrow indicating the order of the nodes. This is missing in the member list.
  • It should be possible to change the displayed order manually.
Last edited 5 weeks ago by GerdP (previous) (diff)

comment:7 Changed 3 weeks ago by simon04

Could you create an annotated screenshot to illustrate whatcan/should be improved? It's not obvious.

Changed 3 weeks ago by GerdP

Attachment: snip.png added

comment:8 Changed 3 weeks ago by GerdP


1) The marked "Down" symbol next to the Nodes symbol should trigger some kind of reverse action when clicked, so that the compare is done with the reversed order. Not sure if it makes sense to click on the right one, I think the left is the reference.
2) The symbol (and the action) is missing in the members dialog.

comment:9 Changed 3 weeks ago by simon04

Description: modified (diff)

comment:10 Changed 3 weeks ago by simon04

Resolution: fixed
Status: reopenedclosed

In 16456/josm:

fix #19161 - History/RelationMemberListViewer: show reversed diff indicator


Last edited 3 weeks ago by simon04 (previous) (diff)

Changed 3 weeks ago by simon04

comment:11 in reply to:  8 Changed 3 weeks ago by simon04

Replying to GerdP:

1) The marked "Down" symbol next to the Nodes symbol should trigger some kind of reverse action when clicked, so that the compare is done with the reversed order. Not sure if it makes sense to click on the right one, I think the left is the reference.

Allowing the user to manually change the sorting of any of the two tables, and taking the possibly reversed diff output into account, sounds difficult to get right and super error prone for (in my opinion) little value gained.

comment:12 Changed 3 weeks ago by simon04

Milestone: 20.05

comment:13 Changed 3 weeks ago by GerdP

What do you think about a new column for the position of the node/member? Might make it clearer that the order is changed.

comment:14 Changed 3 weeks ago by simon04

Indeed, this would make it very clear and should be also adopted for the NodeListViewer of ways.

I've created a separate ticket for this (somewhat independent) enhancement: #19255

Last edited 3 weeks ago by simon04 (previous) (diff)

comment:15 Changed 3 weeks ago by simon04

Summary: Comparison of versions in history is brokenHistory browser: relation member table lacks reversed diff incidator

comment:16 Changed 11 days ago by skyper

Description: modified (diff)

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.