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(commands): add clear_registers command #4408

Closed
wants to merge 0 commits into from

Conversation

chorcheus
Copy link
Contributor

Hi,
there has been a feature request for an option to delete registers, so I politely created this. Thank you for your review.

Closes #4256

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Instead of this being a new command, I think this should be implemented as a special key in the on_next_key callback of select_register. So you would clear registers by pressing " and then the key to clear all registers. I'm not sure what the keybind should be though

@geometryolife
Copy link

Instead of this being a new command, I think this should be implemented as a special key in the on_next_key callback of select_register. So you would clear registers by pressing " and then the key to clear all registers. I'm not sure what the keybind should be though

Perhaps using ”| is a good choice.

@chorcheus
Copy link
Contributor Author

Instead of this being a new command, I think this should be implemented as a special key in the on_next_key callback of select_register. So you would clear registers by pressing " and then the key to clear all registers. I'm not sure what the keybind should be though

Perhaps using ”| is a good choice.

This could get tricky on different keyboard layouts - for example mine has it on opposite sides of keyboard. It could work if this key mapping was overridable - but for that one still needs a separate command to be called.

@kirawi kirawi added A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Oct 25, 2022
@chorcheus
Copy link
Contributor Author

chorcheus commented Nov 9, 2022

Instead of this being a new command, I think this should be implemented as a special key in the on_next_key callback of select_register. So you would clear registers by pressing " and then the key to clear all registers. I'm not sure what the keybind should be though

Perhaps using ”| is a good choice.

This could get tricky on different keyboard layouts - for example mine has it on opposite sides of keyboard. It could work if this key mapping was overridable - but for that one still needs a separate command to be called.

Another possible approach would be to create Registers mode in which one could select register or clear them. For convenience the select register command still could be called outside this mode too as it is now.

Simply binding one key to command after key select_register is pressed does not seem right since register keys should be pressed in that moment.

@chorcheus
Copy link
Contributor Author

@the-mikedavis @geometryolife Thank you for all suggestions, I implemented a solution. I'd be thankful for the review since I am still not sure about the keymapping.

@chorcheus
Copy link
Contributor Author

The work has been moved to #5695; this PR's source branch is master, which isn't convenient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete registers
4 participants