[patch] Join node to way should operate on all selected nodes

[*Martin*]

Join node to way should operate on all selected nodes. Please check my patch. Not sure if it is the best approach but it works and also obeys the undo buffer.

[*Martin*]

the patch

[bastiK]

Patch looks ugly, are you sure it has to be done like that?

[*Martin*]

What's your problem with it? Do you have any better solution already? Next time I'll only report the issue and leave my ugly patches for my own. Good luck.

[bastiK]

Sorry for the rude language, it wasn't my intention to offend you.

It's a weakness of the current undo/redo system, that you cannot build on previous commands and still keep it grouped in one SequenceCommand, it has to be done "in one go". So rather than calling executeCommand(), I would suggest to compile the final list of way nodes directly. Then, one ChangeCommand for each way will be enough.

[*Martin*]

Replying to bastiK:

Sorry for the rude language, it wasn't my intention to offend you.

Offending my code offends me ;-)

It's a weakness of the current undo/redo system, that you cannot build on previous commands and still keep it grouped in one SequenceCommand, it has to be done "in one go".

This is exactly why I "hacked" (extended) SequenceCommand.

So rather than calling executeCommand(), I would suggest to compile the final list of way nodes directly. Then, one ChangeCommand for each way will be enough.

This was my first implementation. It was almost working, but the problem was that if there were multiple nodes joined to the same way segment, then their order was non-deterministic. I believe this is possible to solve too, but it is still much more difficult than my first "hacky" solution that emulates something like "transactions"...

I think it would be good to have something like a "transaction system", where it would be possible to submit changes in any phase of the process so that subsequent operations within same transaction would work on data modified by previous operation. Afterwards the transaction would be commited or rolled back. The commit would also represent one undo unit.

This could maybe make even more algorithms simpler because they could then drop the part that collects changes made by previous steps of the whole action to finally create consistent changes.

In any case I can still (try) to re-implement it to the way you suggest if this feature is desired (for me it is). What do you think?

[bastiK]

Replying to martin.zdila@…:

Replying to bastiK:
This was my first implementation. It was almost working, but the problem was that if there were multiple nodes joined to the same way segment, then their order was non-deterministic. I believe this is possible to solve too, but it is still much more difficult than my first "hacky" solution that emulates something like "transactions"...

I think it would be good to have something like a "transaction system", where it would be possible to submit changes in any phase of the process so that subsequent operations within same transaction would work on data modified by previous operation. Afterwards the transaction would be commited or rolled back. The commit would also represent one undo unit.

This could maybe make even more algorithms simpler because they could then drop the part that collects changes made by previous steps of the whole action to finally create consistent changes.

In any case I can still (try) to re-implement it to the way you suggest if this feature is desired (for me it is). What do you think?

I'd say it would be an improvement and the feature is welcome. You can either do it "oldschool" or it might really be a good idea to extend the undo/redo system. In this case you probably have to keep in mind multithreading, so different "execution sequences" don't get interleaved in an unpredictable way.

[Don-vip]

Ticket #7897 has been marked as a duplicate of this ticket.

[cymerio]

My implementation of the feature

[cymerio]

I've attached my implementation of the feature. I was in need of such feature and couldn't find any patch that applies with the current code base, so I implemented it in my own way.
It was my first time programming in Java, so it may contain mistakes. Please review.
It has some little inefficiencies, but works pretty fast for me.

It works the same as before for 1 node selected (including limiting the joining operation to selected ways).
For multiple nodes, it works joining the nodes all at once, not one after another. Consider the case when joining the first node modifies the shape of the way, then the second node is now so far from the way that can not snap to it. This patch doesn't show that problem: it computes first how it is going to join all the nodes, then at a later stage it makes all the modifications to the ways/nodes.

Note: this works really great to clean some types of geometry data, like the new Spanish Catastro datasource!

[bastiK]

In 7302/josm:

applied #7239 - Join node to way should operate on all selected nodes (patch by cymerio)

[bastiK]

Great, thanks! I only did some minor cosmetic changes.