Fixed #64679 - Delete Lines now works with multiple selections on multiple lines#67287
Fixed #64679 - Delete Lines now works with multiple selections on multiple lines#67287alexdima merged 3 commits intomicrosoft:masterfrom
Conversation
alexdima
force-pushed
the
fix-64679
branch
2 times, most recently
from
b80c785 to
1dc241a
Compare
alexdima
left a comment
alexdima
left a comment
There was a problem hiding this comment.
A few things that are not 100% good:
editor.executeEditsis invoked with an empty array of edits, effectively acting as if it were a call toeditor.setSelections.- After running this action, we are now always going to 1 cursor. This is not good, e.g. when having one cursor per line -- I would expect to have here 3 cursors at the end:
| Before | After |
|---|---|
 |
 |
Also, another case that is not great is when having two clusters of multiple cursors -- I would expect to have here 2 cursors at the end:
All in all, tweaking the resulting selection is not the approach I would take. I would generate IIdentifiedSingleEditOperation and drop the entire DeleteLinesCommand, i.e. I would loop over all selections, and collect all lines to be deleted. I would make sure to dedup them (like a set reunion) and then issue clear edits that never overlap. I would then insert a cursor under each such deletion.
alexdima
changed the title
|
Thanks for the feedback, I'll go back and make the appropriate changes |
irrationalRock
force-pushed
the
fix-64679
branch
from
1dc241a to
cddb02d
Compare
|
So made the changes you requested. I removed DeleteLinesCommand entirely and now issue edits to remove text. Here is a couple of gif to show the changes. Three Cursors: Two Lines with Multiple Cursors: My branch needs some work done on it but I wanted to see if the cursors locations after |
alexdima
changed the title
|
👍 Thank you! It was almost perfect, I just needed to add some accounting for the deleted lines in establishing the resulting cursor selection. |
2 participants
This PR fixes #64679.
It now collapses all cursors:
Also, now deletes multiple lines with multiple selections:
Thanks for considering this request.