Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#3520 closed enhancement (fixed)

Do not remove selection when using undo

Reported by: Pekka Lampila <pekka.lampila@…> Owned by: team
Priority: minor Milestone:
Component: Core Version:
Keywords: undo selection Cc:


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.

Attachments (0)

Change History (11)

comment:1 Changed 14 years ago by bastiK

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

comment:2 Changed 14 years ago by bastiK

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

comment:3 Changed 14 years ago by 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.

comment:4 Changed 14 years ago by 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.

comment:5 Changed 14 years ago by stoecker

Resolution: fixed
Status: newclosed

Fixed in r2541.

comment:6 Changed 14 years ago by 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).

comment:7 Changed 14 years ago by bastiK

Resolution: fixed
Status: closedreopened

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?

comment:8 Changed 14 years ago by bastiK

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

comment:9 Changed 14 years ago by stoecker

Resolution: fixed
Status: reopenedclosed

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.

comment:10 Changed 14 years ago by bastiK

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

comment:11 Changed 14 years ago by 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.)

Modify Ticket

Change Properties
Set your email in Preferences
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.