[Patch] conflict detection not working for combine way with one null and one tag set

[dieterdreist]

Usually when you combine ways with different attributes, JOSM asks you which value to choose (or delete or merge values), but if one way has not set a tag and the other does, there is no warning and the tag will be silently set for the combined way.

[stoecker]

Yes. That's intended behaviour.

I which situation does that cause trouble?

[SekeRob]

Observing that too, BUT, I've considered that intentional inheritance. i.e. if one has surface=asphalt and the other does not have a surface tag then both get surface=asphalt with which I'm fine. When tags have different values than the warning screen needs to appear.

[dieterdreist]

It can cause trouble in many situations, when attributes get extended to places where they do not belong to (e.g. bridge=yes). I understand the current behaviour is what mappers are used to, so likely it should be configurable ("do not ask again").

The use case is for example combining ways of the same kind of objects (e.g. representing route relations) but where some attributes are different (e.g. note or description, where the content of the fields is not structured), some tag values prevent merging while others do not, so I want to check them manually.

Before combining one can see it in the tags panel when there is only empty values and a single value, but it is tiring and error prone with long tag lists, having a warning message popup for any tag differences on merge would be safer, as it is with different values.

An exception should be made for merging a way without any tags (no warning in this case).

[stoecker]

Hmm, Ah you don't mean untagged, but differently tagged. Hmm, I thought in this case a conflict dialog should popup. Seems it doesn't.

[skyper]

Duplicate of #18597?

[stoecker]

Seems so. Any idea why we restricted that to non-expert users? It's a long time ago and situation was different then, but I still don't know why...

[dieterdreist]

Yes, looks like a duplicate. I am in expert mode and I do get the warning for different values and the same tag, but not if the tag is missing from the other objects

[skyper]

Replying to stoecker:

Any idea why we restricted that to non-expert users? It's a long time ago and situation was different then, but I still don't know why...

I have no glue and Simon did not have either four years ago.

For expert, it could be still optional with a dialog to remember the choice for the session or in general.
Only for objects without any tag or membership the tags/memberships conflict dialog should be skipped (comment 7 on #7513).

[dieterdreist]

I just had the idea that for multiselections, keys with different values could be additionally highlighted in the tags window. You are already doing a good job by using italics, but maybe the background could be yellow in addition? (would be an enhancement and is not a bug, should I open a new ticket?)

[stoecker]

Replying to skyper:

For expert, it could be still optional with a dialog to remember the choice for the session or in general.
Only for objects without any tag or membership the tags/memberships conflict dialog should be skipped (comment 7 on #7513).

Could be, but really I see little advantage. The chance to destroy data is extremely high. The benefits low. Don't even think that was different years ago.

Maybe: Enable for Experts and when too many complain make it optional ;-)

Replying to dieterdreist:

would be an enhancement and is not a bug, should I open a new ticket?)

For sure. That's another topic.

[taylor.smock]

Ticket retargeted after milestone closed

[taylor.smock]

Ticket retargeted after milestone closed

[GerdP]

I don't understand what this ticket is about, exactly.

[taylor.smock]

Ticket retargeted after milestone closed

[skyper]

Replying to GerdP:

I don't understand what this ticket is about, exactly.

This ticket is probably a duplicate of #18597 and about (optionally) enabling the tags/memberships conflict in expert mode also if an identical tag is present only on one/some object(s) and empty on others.

See also #7513 and #18675 for more problems.

[dieterdreist]

Replying to skyper:

Replying to GerdP:

I don't understand what this ticket is about, exactly.

This ticket is probably a duplicate of #18597 and about (optionally) enabling the tags/memberships conflict in expert mode also if an identical tag is present only on one/some object(s) and empty on others.

See also #7513 and #18675 for more problems.

Yes, it looks like a duplicate of #18597. IMHO there is a tags conflict in the "combine ways" context generally if tags are different, and a tag that is not there on some objects should raise a warning on combine by default. The expert option would be to be able to ignore it. The exception that doesn't raise a warning should be ways without any tags

[GerdP]

OK, I understand that it can be problematic to silenty combine two ways where one has highway=path and the other highway=path+fixme=xyz. Maybe not so much if one has highway=path and the other highway=path+surface=ground, but that's a different problem.
What exactly should happen? My understanding: Forget the warning popup, just show the conflict dialog unless all tags of all tagged ways are equal?
Should there be a preference to restore the current behaviour?

[skyper]

As the behavior for expert mode is like that since years some users might be used to it. So I would say a preference to restore current behavior would be nice.

Yes, just show the same tags/memberships conflict dialog in both ("normal" and expert) mode. Keep in mind that this counts for all actions which combine/merge objects, e.g. combine ways, merge nodes, join areas and possibly more.

[GerdP]

possible conflict dialog when merging two ways, one with highway=path, the other with highway=path + surface=asphalt

[GerdP]

non-expert popup

[GerdP]

Assume you combine two ways, one with highway=path, the other with highway=path + surface=asphalt, both not members of any relation.

How should the conflict dialog look like? With my work-in-progress patch it would look like this:

Is this enough if we really remove this popup for good which normally precedes the dialog for non-experts?

By the way: We have two non-expert dialogs which might pop up before the conflict dialog (not counting the reverse direction dialog).
One warns about tag conflicts, the other about relation membership conflicts. Both(!) are disabled with the same preference combine_tags

  <tag key='message.combine_tags' value='false'/>
  <tag key='message.combine_tags.value' value='0'/>

That was probably not intended?

[dieterdreist]

the dialog should be just the same as if both had different attributes (indeed, not having an attribute is different to having it with any value)

[GerdP]

conflict without prepared solution

[GerdP]

So, you would want this popup where the user has to choose the wanted value?

[dieterdreist]

yes!

[GerdP]

The patch

• removes all the code to show the special non-expert dialogs
• adds code to check if a conflict exists where one or more tagged ways don't have the same tag value and - if so - show the conflict dialog.
• adds a preference combine-conflict-precise . If set to false the old behaviour is restored.

I think the new method getResolvableKeys() is straightforward but it somehow bypasses all the logic in the conflict resolver code.

I've noticed that there is special code to handle tags like step_count or capacity. I'll have to verify that this patch doesn't break anything here. Edit: Works as before.

Possible problem: Non-expert users may have decided to suppress the dialogs with decision cancel, so that combine ways silently refuses to work when there is any conflict. Now they will see the complex conflict dialog instead. So maybe better change the patch to keep the non-expert dialogs?

[GerdP]

alternative patch which doesn't remove the non-expert dialogs

[GerdP]

The alternative patch also changes the preference for the non-expert relation member ship warning popup from combine_tags to combine_relation_member.

[GerdP]

In 18988/josm:

fix #23305: conflict detection not working for combine way with one null and one tag set
(23305-alt.patch)

• adds code to check if a conflict exists where one or more tagged ways don't have the same tag value and - if so - show the conflict dialog.
• adds a preference combine-conflict-precise . If set to false the old behaviour is restored.
• changes the preference for the non-expert relation member ship warning popup from combine_tags to combine_relation_member, so that both dialogs have separate settings

[GerdP]

In 19002/josm:

see #23305: correct @since xxx

[GerdP]

In 36216/osm:

see #23305: adapt unit test for new preference combine_relation_member

[NieWnen]

This change is somewhat controversial, and I would say it is a "breaking change" as far as plugins are concerned.
Already at least a few mappers have asked and pointed out that mapping is now more time-consuming because of these conflicts.

E.g. Utilsplugin2 has been affected by this and all plugins that use it (including my building plugin for the Polish community plbuildings). Non-conflicting tags when merging objects show a conflict dialog to choose between an empty value or a filled one....

I already recommended mappers, to change above setting combine-conflict-precise to false, but I am not convinced about the patch itself, or its default setting value.

[GerdP]

Yes, this change turned out to be quite problematic. See also #23555.

[dieterdreist]

I believe it is right to show a conflict where a tag is set on one and empty on the other, but I can also understand that people have become used to josm not doing it, so it can encumber some workflows. If it is configurable everybody could be served.

[GerdP]

In 19022/josm:

see #23305 and #23555:
-revert most of the changes in r18988, it caused too many problems with plugins which relied on the old behaviour

[anonymous]

will it still be possible to opt in for conflict detection to detect a conflict when merging objects with different tagging (tags not on one)?

[GerdP]

Well, we can also revert the revert and just change the default for the combine-conflict-precise preference. In that case we probably have to find a solution for the switches in the popup dialog as they are no longer working as expected. See ticket:23555#comment:16

[skyper]

I think the intention to consider keys with identical value only on one (some) object when merging/combining is good and there should be no differences between expert and "normal" mode. Unfortunately the change was not 100% convincing with the sudden change of well-established behavior and the little flaws like the non-working checkboxes.

[dieterdreist]

maybe this ticket should be reopened? It was almost fixed but is now back into regression (no warning when extending tags by combining objects)