Do not remove selection when using undo

[Pekka Lampila <pekka.lampila@…>]

Selection should not be removed when using undo.

One example where this would be nice is drawing a way with multiple nodes. If you misplace a node and press undo to remove it the selection is lost.

[bastiK]

(In [2539]) see #3520 - Do not remove selection when using undo (partial fix)

[bastiK]

This is an easy preliminary fix. It restores only one selection when undo is invoked multiple times in a row.

[stoecker]

I don't think what you did is good. You restore selections and this is nothing we want have.

The bug author only wanted that the active selection is not lost when undoing. That's an entirely different topic. You can remove the global list then and only change the code in undo() function.

[bastiK]

Are you sure? I think it is exactly what he wants.

Consider Actions like delete, merge or unglue. It makes not much sense to keep selection for these

"Undo" means to restore the state of the program to some earlier point in time. That is what i do with this.

[stoecker]

Fixed in r2541.

[stoecker]

We have already a selection history for this. Mixing this into normal history is not good for several reasons.

At least what I read from above is that e.g. when you have a bit more complicated selection and you make an mistake, then after undo you need to redo the whole selection. With the little bit adapted patch this is now no longer necessary, as undo does no longer kill active selection (which has probably been introduced to overcome some consistency problems).

[bastiK]

Well I agree with the first point and that is why it is preliminary. The proper use of selection history touches too much code to go into tested.

For the latter paragraph, you may be arguing in my favor. :)

Imagine you selected a lot of nodes and delete them. Then you realize it was one node too much. Currently undo would destroy the selection and you have to start all over again. My version would restore the selection.

Similar with merge.

Could you explain in what situations my patch would be wrong / confusing?

[bastiK]

...When undoing merge only the target node will be selected. At least this is a bit strange currently.

[stoecker]

You can use selection history to get previous selection in the cases you describe. Now it is a don't touch my current selection except you need to. I think this is how it should be done. Previously we had a "always kill selection" which was unfine (as e.g. we don't do that for redo).

The problem with you patch is, that it is inconsistent. Selection will be undone with other actions, but selections inbetween will not be. And I really don't want to have all selections in the command history. Then we would need to have no longer a "command history", but an action history and this is overkill especially in terms of memory usage.

[bastiK]

(In [2552]) cleanup for r2539 and r2541. See #3520

[bastiK]

The previous code was quite useless for preserving the selection. One can rely on DataSet to keep the selection state consistent. (But it won't fire selection change when it does these hygiene checks.)