Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEAT] Create a cursor on jumplist entry #2808

Closed

Conversation

carrascomj
Copy link

Related to #2784 (comment)

This PR adds a command copy_cursor_backward that creates a new cursor by jumping backward in the jumplist, preserving the previous selection. This adds a flexible way of editing text with multiple cursors, especially when combined with textobjects motions.

Demo

Save selection to jumplist C-s, jump to function ]f, copy cursor backward C-q:
copy_back

TODO

  • Keymap, I have assigned C-q for no particular reason.
  • Docs.
  • Copy cursor forward comand? The implementation is trivial but I don't see a lot of purpose in that.

@EpocSquadron
Copy link
Contributor

This is a pretty neat idea! What happens if the jumplist switches to another buffer? I would argue the forward equivalent isn't a bad idea to include, if you overshoot the jumplist and need to go forward.

@carrascomj
Copy link
Author

What happens if the jumplist switches to another buffer?

It panics 😅. Sorry, I hadn't checked, I will look into into it and maybe add a test if I figure out how to cover that.

Regarding the forward variant, I have tried and it is fine. I would not use it a lot but if this is merged, a user will ask for it at some point; I think I'll add it.

@the-mikedavis
Copy link
Member

Keymap-wise, I'm not a big fan of C-q. It looks like C-m is interpreted by crossterm as Enter so that's out. My terminal emulator is eating C-O altogether but maybe that's just my config. I'll think about it some more. Another option is to not add a default keybind and let people create a keybind if they want this.

For docs, I've been meaning to add a section to book/src/usage.md on the jumplist. This could fit there and possibly in the tutor (runtime/tutor.md) as well.

Forward variant: we could implement it and not bind it if we don't think it will be useful. That way someone can bind it themselves if they find a use for it without needing to change the code.

@carrascomj
Copy link
Author

I have fixed the problems with jumping to another buffer, removed the keymap (C-O was not working either for me) and added the forward variant without a keymap.

Anyways, this PR may not be worth it if marks are a desired feature.

let count = cx.count();
let view = view_mut!(cx.editor);
let (view, doc) = current!(cx.editor);
let mut this_selection = doc.selection(view.id).clone();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clone is not required if copy_cursor is true, could it be avoided in some elegant way?

@the-mikedavis
Copy link
Member

this PR may not be worth it if marks are a desired feature

yeah I was thinking similar: marks seem to be a cleaner approach to this, especially if we had the rich operations from kakoune's A-z/A-Z https://github.com/mawww/kakoune/blob/6565f6edd700a1490c670f4e60cd52f7f78dbbe5/doc/pages/keys.asciidoc#marks

It looks like @archseer is interested in tackling marks because of some prior experience with it in nvim but it might not be a priority for him at the moment #703

@carrascomj
Copy link
Author

Closing in favor of a future implementation of marks.

@carrascomj carrascomj closed this Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants