Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dont delete nodes on undo join ways #239

Merged

Conversation

tunp
Copy link
Contributor

@tunp tunp commented May 30, 2021

When joining ways and then doing undo AddFeatureCommand deletes nodes if joined ways were not on different layers. This then breaks any uploading to OSM.
This pull request simply doesn't do AddFeatureCommand if ways are not on different layers.

@Krakonos
Copy link
Member

I don't think I understand the problem correctly, too many negations to wrap my head around. Can you provide an example?

I also think doing operations on features on different layers may be the culprit and we should possibly catch it there, though I think the problem you mentions happens if those features are on the same layer.

@tunp
Copy link
Contributor Author

tunp commented May 31, 2021

Sorry for the unclear description.
You are right. This problem only happens when Features are on same layer. Everything seems to work well when joining ways on different layers.

Here's an example:

  1. Create two ways that are connected together by a one node. These ways should be on same layer.
  2. Join the ways so it becomes a one way.
  3. Do undo.
  4. Try to upload to server. It should give a reference error.

@Krakonos
Copy link
Member

Interesting. I would have guessed re-adding the node would cause more issues than in undo. But your fix looks reasonable, merging that for 0.19. Thanks!

@Krakonos Krakonos merged commit 9be50e5 into openstreetmap:master Aug 11, 2021
@Krakonos Krakonos added this to the 0.19.0 milestone Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants