Changeset 6652 in josm for trunk

Timestamp:

2014-01-06T20:42:32+01:00 (10 years ago)

Author:

simon04

Message:

fix #9525 - multiselecting objects does not show differing relation membership correctly in the sidebar

For instance, positions are displayed as 48-49,91,✗.

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialog.java (modified) (6 diffs)
• src/org/openstreetmap/josm/tools/Predicates.java (modified) (2 diffs)
• src/org/openstreetmap/josm/tools/Utils.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/tools/UtilsTest.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialog.java

@@ -24 +24 @@
 import java.util.EnumSet;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -95 +96 @@
 import org.openstreetmap.josm.tools.LanguageInfo;
 import org.openstreetmap.josm.tools.OpenBrowser;
+import org.openstreetmap.josm.tools.Predicates;
 import org.openstreetmap.josm.tools.Shortcut;
 import org.openstreetmap.josm.tools.Utils;
@@ -321 +323 @@
                 boolean isDisabledAndHidden = (((Relation)table.getValueAt(row, 0))).isDisabledAndHidden();
                 if (c instanceof JLabel) {
-                    JLabel label = (JLabel)c;
-                    MemberInfo col = (MemberInfo) value;
-
-                    String text = null;
-                    for (RelationMember r : col.role) {
-                        if (text == null) {
-                            text = r.getRole();
-                        }
-                        else if (!text.equals(r.getRole())) {
-                            text = tr("<different>");
-                            break;
-                        }
-                    }
-
-                    label.setText(text);
+                    JLabel label = (JLabel) c;
+                    label.setText(((MemberInfo) value).getRoleString());
                     if (isDisabledAndHidden) {
                         label.setFont(label.getFont().deriveFont(Font.ITALIC));
@@ -623 +612 @@
                     MemberInfo mi = roles.get(r);
                     if(mi == null) {
-                        mi = new MemberInfo();
+                        mi = new MemberInfo(newSel);
                     }
                     roles.put(r, mi);
@@ -792 +781 @@
 
     static class MemberInfo {
-        List<RelationMember> role = new ArrayList<RelationMember>();
-        List<Integer> position = new ArrayList<Integer>();
+        private List<RelationMember> role = new ArrayList<RelationMember>();
+        private Set<OsmPrimitive> members = new HashSet<OsmPrimitive>();
+        private List<Integer> position = new ArrayList<Integer>();
+        private Iterable<OsmPrimitive> selection;
         private String positionString = null;
+        private String roleString = null;
+
+        MemberInfo(Iterable<OsmPrimitive> selection) {
+            this.selection = selection;
+        }
+
         void add(RelationMember r, Integer p) {
             role.add(r);
+            members.add(r.getMember());
             position.add(p);
         }
+
         String getPositionString() {
             if (positionString == null) {
-                Collections.sort(position);
-                positionString = String.valueOf(position.get(0));
-                int cnt = 0;
-                int last = position.get(0);
-                for (int i = 1; i < position.size(); ++i) {
-                    int cur = position.get(i);
-                    if (cur == last + 1) {
-                        ++cnt;
-                    } else if (cnt == 0) {
-                        positionString += "," + cur;
-                    } else {
-                        positionString += "-" + last;
-                        positionString += "," + cur;
-                        cnt = 0;
-                    }
-                    last = cur;
-                }
-                if (cnt >= 1) {
-                    positionString += "-" + last;
-                }
+                positionString = Utils.getPositionListString(position);
+                // if not all objects from the selection are member of this relation
+                if (Utils.exists(selection, Predicates.not(Predicates.inCollection(members)))) {
+                    positionString += ",\u2717";
+                }
+                members = null;
+                position = null;
+                selection = null;
             }
             if (positionString.length() > 20) {
@@ -826 +813 @@
             }
             return positionString;
+        }
+
+        String getRoleString() {
+            if (roleString == null) {
+                for (RelationMember r : role) {
+                    if (roleString == null) {
+                        roleString = r.getRole();
+                    } else if (!roleString.equals(r.getRole())) {
+                        roleString = tr("<different>");
+                        break;
+                    }
+                }
+                role = null;
+            }
+            return roleString;
+        }
+
+        @Override
+        public String toString() {
+            return "MemberInfo{" +
+                    "roles='" + roleString + '\'' +
+                    ", positions='" + positionString + '\'' +
+                    '}';
         }
     }

trunk/src/org/openstreetmap/josm/tools/Predicates.java

@@ -3 +3 @@
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 
+import java.util.Collection;
 import java.util.regex.Pattern;
 
@@ -84 +85 @@
         };
     }
+
+    /**
+     * Returns a {@link Predicate} executing {@link Collection#contains(Object)}.
+     */
+    public static <T> Predicate<T> inCollection(final Collection<? extends T> target) {
+        return new Predicate<T>() {
+            @Override
+            public boolean evaluate(T object) {
+                return target.contains(object);
+            }
+        };
+    }
 }

trunk/src/org/openstreetmap/josm/tools/Utils.java

@@ -37 +37 @@
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
@@ -861 +862 @@
 
     /**
+     * Returns a human readable representation of a list of positions.
+     * <p/>
+     * For instance, {@code [1,5,2,6,7} yields "1-2,5-7
+     * @param positionList a list of positions
+     * @return a human readable representation
+     */
+    public static String getPositionListString(List<Integer> positionList)  {
+        Collections.sort(positionList);
+        final StringBuilder sb = new StringBuilder(32);
+        sb.append(positionList.get(0));
+        int cnt = 0;
+        int last = positionList.get(0);
+        for (int i = 1; i < positionList.size(); ++i) {
+            int cur = positionList.get(i);
+            if (cur == last + 1) {
+                ++cnt;
+            } else if (cnt == 0) {
+                sb.append(",").append(cur);
+            } else {
+                sb.append("-").append(last);
+                sb.append(",").append(cur);
+                cnt = 0;
+            }
+            last = cur;
+        }
+        if (cnt >= 1) {
+            sb.append("-").append(last);
+        }
+        return sb.toString();
+    }
+
+
+    /**
      * Returns a list of capture groups if {@link Matcher#matches()}, or {@code null}.
      * The first element (index 0) is the complete match.

trunk/test/unit/org/openstreetmap/josm/tools/UtilsTest.java

@@ -5 +5 @@
 import org.junit.Test;
 import org.openstreetmap.josm.Main;
-import org.openstreetmap.josm.data.Preferences;
 
 import java.io.BufferedReader;
 import java.net.URL;
+import java.util.Arrays;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
 
 /**
@@ -87 +90 @@
         x.close();
     }
+
+    @Test
+    public void testPositionListString() throws Exception {
+        assertThat(Utils.getPositionListString(Arrays.asList(1)), is("1"));
+        assertThat(Utils.getPositionListString(Arrays.asList(1, 2, 3)), is("1-3"));
+        assertThat(Utils.getPositionListString(Arrays.asList(3, 1, 2)), is("1-3"));
+        assertThat(Utils.getPositionListString(Arrays.asList(1, 2, 3, 6, 7, 8)), is("1-3,6-8"));
+        assertThat(Utils.getPositionListString(Arrays.asList(1, 5, 2, 6, 7)), is("1-2,5-7"));
+    }
 }