[Patch] Indentation/Block mismatch – maybe cause for wrong “modified” warnings (or * in window title)?

[xeen]

Have a look at source:josm/trunk/src/org/openstreetmap/josm/command/ChangePropertyCommand.java#L131​:

for (OsmPrimitive osm : objects) {
    boolean modified = false;
     // loop over all tags
    for (Map.Entry<String, String> tag : this.tags.entrySet()) {
        String oldVal = osm.get(tag.getKey());
        String newVal = tag.getValue();
        if (newVal == null || newVal.isEmpty()) {
            if (oldVal != null) {
                osm.remove(tag.getKey());
                modified = true;
            }
        }
        else if (oldVal == null || !newVal.equals(oldVal))
            osm.put(tag.getKey(), newVal);
            modified = true;                                      // this is line 131
    }
    if (modified)
        osm.setModified(true);
}

As far as I can tell this marks all objects as modified which have at least one tag. From the logic of that code I would deduce that the identation is correct, but the braces around the else-if clause are missing.

Since this might introduce hard-to-catch bugs, I’d like someone else to have a look at this. I believe this was introduced in r4773.

[bastiK]

Yes, looks like the braces are missing. I think it doesn't matter because init makes sure that only "modified" objects are taken into account.

[xeen]

Appears you are right and the additional modified work is superfluous. Suggested patch.

[simon04]

bastiK, xeen: Shall this patch be applied?

[bastiK]

I see nothing wrong with this patch.

[xeen]

In 5143/josm:

fix #7504 -- Remove superfluous check

[xeen]

thanks, had this almost forgotten…