Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

🐧 🍎 🏁 Added meta-alt-e hotkey for replac… #713

Merged
merged 1 commit into from
Jan 6, 2017

Conversation

ruthgrace
Copy link
Contributor

…e-next, as requested in #67

For Mac, the hotkey is cmd-alt-e. For Windows/Linux, I've used ctrl for the meta key, so the hotkey is ctrl-alt-e. Normally the meta key isn't ctrl on Windows/Linux, but it seems that ctrl is used as the meta key throughout the find-and-replace module.

Please let me know if you have any feedback on this pull request. Thanks in advance!
Ruth

@ruthgrace
Copy link
Contributor Author

Hi @50Wliu, I couldn't find any examples of how to write a test for a hotkey in the spec folder (but maybe I'm missing something), and I'm not sure how to do it because the hotkeys are different for Mac vs Windows/Linux. The replace-next function was already implemented and already has tests. Would you be able to point me to an example of a test for a hotkey somewhere in the atom codebase for me to reference?

@50Wliu
Copy link
Contributor

50Wliu commented May 9, 2016

@ruthgrace the needs-testing label just means that someone needs to test this out and make sure it works properly. If we wanted you to write specs we would tell you that directly :).

@ruthgrace
Copy link
Contributor Author

Thanks for the clarification! :)

@lee-dohm
Copy link
Contributor

lee-dohm commented Jul 7, 2016

It looks like the default for Replace Next on Sublime for Windows is Ctrl+Shift+H. Should we use that instead on Windows and Linux? Or would that conflict with something else?

screen shot 2016-07-07 at 3 24 35 pm

@lee-dohm lee-dohm self-assigned this Jul 7, 2016
@maxbrunsfeld
Copy link
Contributor

Thanks @ruthgrace! ctrl-alt-keybindings are a bit controversial on windows, because ctrl-alt is also used for typing certain characters on some keyboard layouts. I'm going to merge this but just remove the windows/linux key-binding.

@damieng - as our windows expert, I invite you to add back a binding for this command if you feel so inclined.

@maxbrunsfeld maxbrunsfeld merged commit 1fa3b5e into atom:master Jan 6, 2017
@damieng
Copy link
Contributor

damieng commented Jan 6, 2017

Yeah I'm okay with this not having a shortcut. If we look at the shortcuts already assigned there's all sort of probably rarely-used commands bound.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants