zed-industries / zed

Code at the speed of thought – Zed is a high-performance, multiplayer code editor from the creators of Atom and Tree-sitter.
https://zed.dev
Other
50.3k stars 3.1k forks source link

Restore cursor position if selection is canceled #8588

Open failable opened 8 months ago

failable commented 8 months ago

Check for existing issues

Describe the feature

Currently when "selecting larger/smaller syntax node", the cursor is moved to the end of the selection. If one then cancel the selection, the cursor is left at the end of the original selection.

I wonder if it is better to restore the cursor position before starting selection automatically, because I find that when I cancel selection, I need to move back to the original position quite often which is a bit inconvenient especially when the selection is expanded larger than the visible window. "GoBack" does not work in this situation, and I personally prefer it's done automatically when selection is canceled to avoid polluting the "GoBack" stack.

How do you think?

If applicable, add mockups / screenshots to help present your vision of the feature

No response

SomeoneToIgnore commented 8 months ago

Oh, sounds like a duplicate of your other issue? https://github.com/zed-industries/zed/issues/8105 That would be great to add and I think might not be very hard so start, at least.

https://github.com/zed-industries/zed/blob/225dd0f9a0aff5c21f3f2208548438acfeaa9b5a/crates/editor/src/editor_tests.rs#L3712-L3739

shows how to init a test buffer with text, then track carets in it ˇ and, below, selection ranges: def«ˇg»hi Something similar can be added to test the SelectAll -> Cancel test with carets, and there will be problems:

Maybe, it's fine to keep the selection mechanism working here this way, all existing selections work so that they change the caret position. Maybe it's not, some design/justification is needed there.

On that place, there's self.selection_history available that should have the previous caret, but try_cancel's self has no access there — we need to think whether we should set that previous caret to MutableSelectionsCollection or take it from editor's SelectionHistory, or do something else clever — so far it seems that picking the old one from the selection stack is fine, as long as we do not remove it from there.

SomeoneToIgnore commented 8 months ago

I've added the good first issue here but be wary of the fact that some design of the vague points will be needed — nonetheless, it still looks pretty actionable and rather standalone to work on it.

failable commented 8 months ago

😢I just can not believe I forget I opened a similar issue for it…

nikensss commented 8 months ago

Hi! I would like to try to tackle this issue with @mrnugget next wednesday. He opened up a calendly request to have a 1h session with him during next week, and I managed to make a booking on Wednesday 13th at 15h. Would that be ok? I will be looking at this until then.

nikensss commented 8 months ago

Hi @failable, I have been able to reproduce the issue. Just to confirm, when you say:

cancel the selection

Do you mean as in pressing the <ESC> key? And last question, are we talking about the selections when doing Expand selection (ie, Control + Shift + Right)? Screenshot 2024-03-12 at 13 47 36

failable commented 8 months ago

Do you mean as in pressing the key?

Yes. Not sure if it is menu::Cancel or editor::Cancel single both they are bound to esc.

And last question, are we talking about the selections when doing Expand selection (ie, Control + Shift + Right)?

Yes, I mean commands like "editor::SelectLarger/SmallerSyntaxNode" and editor::SelectAll, as mentioned in the old issue https://github.com/zed-industries/zed/issues/8105

mrnugget commented 8 months ago

So @nikensss and I just looked at this and we're not sure whether we should do this, or how exactly it should work if we were to do it. No other editor (we looked at Sublime, VS Code, Neovim) does restore the cursor position. I also think it might be a "breaking" change for a lot of people, or unexpected behavior.

My thinking here is that everywhere else the selection follows the cursor: if you start a selection and then move your cursor in a way that it gets added to the navigation history, then you can jump back after canceling the selection, it's independent of whether selection was on or not, the selection just follows the cursor.

The proposal here would change this though in that navigation-with-selection would now be a different thing from navigation-without-selection. And there's nothing wrong with that, I'm just not sure how to cleanly add this right now.

SomeoneToIgnore commented 8 months ago

To note, JetBrains IDEs restore the caret position after select all is cancelled, but they behave differently around the selection logic.

As the video tries to show, even when Select All is triggered, the caret is blinking on the previous place, so restoring there is simple. Zed on the test level panics when the caret is not on the edge of the selection.

https://github.com/zed-industries/zed/assets/2690773/b122558d-0e38-4958-a999-584f988fb982

failable commented 8 months ago

There is also something should be noticed.

'Early move': In Emacs, if one invoke select all command, the cursor is moved immediately (to the beginning of the buffer). If one cancel the selection, the cursor is not moved again. I am not saying this is good, but at least a bit better than other editor since the cursor position is a bit 'more predictable' than in other editor.

(Beside this 'early move' way to select things, emacs provides a way (may be extension) to select things (function, class, line, buffer, paragraph etc.) like the video added by @SomeoneToIgnore and does not move the cursor at all.)

'Late move': While in other editors (VSCode, Zed) do not move the cursor at when select all. But if the selection is then canceled, the cursor is then moved. I personally don't see what are the advantages of this move of cancelation.