Modify

Opened 15 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:

Description

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 by bastiK, 14 years ago

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

comment:2 by bastiK, 14 years ago

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

comment:3 by stoecker, 14 years ago

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 by bastiK, 14 years ago

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 by stoecker, 14 years ago

Resolution: fixed
Status: newclosed

Fixed in r2541.

comment:6 by stoecker, 14 years ago

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 by bastiK, 14 years ago

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 by bastiK, 14 years ago

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

comment:9 by stoecker, 14 years ago

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 by bastiK, 14 years ago

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

comment:11 by bastiK, 14 years ago

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
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.