#7504 closed defect (fixed)
[Patch] Indentation/Block mismatch – maybe cause for wrong “modified” warnings (or * in window title)?
Reported by: | xeen | Owned by: | team |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | Core | Version: | |
Keywords: | Cc: | joshdoe, bastiK |
Description
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.
Attachments (1)
Change History (7)
comment:1 by , 12 years ago
by , 12 years ago
Attachment: | 7504.patch added |
---|
Appears you are right and the additional modified work is superfluous. Suggested patch.
comment:2 by , 12 years ago
Summary: | Indentation/Block mismatch – maybe cause for wrong “modified” warnings (or * in window title)? → [Patch] Indentation/Block mismatch – maybe cause for wrong “modified” warnings (or * in window title)? |
---|
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.