Modify

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#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)

7504.patch (1.5 KB) - added by xeen 8 years ago.
Appears you are right and the additional modified work is superfluous. Suggested patch.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 8 years ago by 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.

Changed 8 years ago by xeen

Attachment: 7504.patch added

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

comment:2 Changed 8 years ago by skyper

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)?

comment:3 Changed 8 years ago by simon04

bastiK, xeen: Shall this patch be applied?

comment:4 Changed 8 years ago by bastiK

I see nothing wrong with this patch.

comment:5 Changed 8 years ago by xeen

Resolution: fixed
Status: newclosed

In 5143/josm:

fix #7504 -- Remove superfluous check

comment:6 Changed 8 years ago by xeen

thanks, had this almost forgotten…

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.