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

Optionally dismiss suggestion list on up/down instead of cycling. #838

Merged
merged 3 commits into from Dec 5, 2017

Conversation

Projects
None yet
3 participants
@phyllisstein
Contributor

phyllisstein commented Apr 15, 2017

Description of the Change

I often found myself pining for the behavior of Sublime Text's autocompletion widget, which permitted navigation with one's arrow keys, à la the "Core Movement Commands" setting here, but wouldn't be so presumptuous as to hijack them and would politely yield control and see itself out if one tried to navigate above its first suggestion or below its last. It was especially handy for breaking out of autocompletion without reaching for Escape---one could just hit the up arrow and get on with one's life---but it was generally nice not to spend the occasional confused millisecond trying to make the cursor move when a suggestion list was in view.

Implementing the change was super straightforward. There's a new preference, "Cycle Suggestions": in its presence, the behavior is unchanged; in its absence, trying to move beyond the boundaries of the list dismisses it and fires a new movement in the editor.

2017-04-15 06_42_06

Alternate Designs

I briefly tried it without forcing the cursor up/down a line when the list was dismissed. If anything this was more frustrating than getting caught in a loop, as the keystroke appeared to do nothing at all.

Benefits

Never again will an element I'm usually only half-attending to take over my arrow keys.

Possible Drawbacks

Anyone who enables this feature and dislikes it will lose valuable seconds disabling it again.

Applicable Issues

Apparently I'm all alone here.

@50Wliu 50Wliu added the needs-review label Apr 16, 2017

@leroix

This comment has been minimized.

Show comment
Hide comment
@leroix

leroix Nov 30, 2017

Contributor

Sorry this PR hasn't gotten more attention. It looks like a reasonable change to me. I think I would like to see a more intuitive config setting name and description. I don't think it's totally obvious what this setting does from the current description.

Contributor

leroix commented Nov 30, 2017

Sorry this PR hasn't gotten more attention. It looks like a reasonable change to me. I think I would like to see a more intuitive config setting name and description. I don't think it's totally obvious what this setting does from the current description.

@phyllisstein

This comment has been minimized.

Show comment
Hide comment
@phyllisstein

phyllisstein Dec 1, 2017

Contributor

No worries! Lots of more interesting stuff going on around it 😃. Would something like "Dismiss on keyboard navigation" be a little clearer?

Contributor

phyllisstein commented Dec 1, 2017

No worries! Lots of more interesting stuff going on around it 😃. Would something like "Dismiss on keyboard navigation" be a little clearer?

@leroix

This comment has been minimized.

Show comment
Hide comment
@leroix

leroix Dec 4, 2017

Contributor

Maybe, the setting should be moveToCancel - "Moving up when the first item is selected or down when the last item is selected cancels the suggestion list"?

Contributor

leroix commented Dec 4, 2017

Maybe, the setting should be moveToCancel - "Moving up when the first item is selected or down when the last item is selected cancels the suggestion list"?

@phyllisstein

This comment has been minimized.

Show comment
Hide comment
@phyllisstein

phyllisstein Dec 4, 2017

Contributor

Done and done. Let me know if you have any other suggestions!

Contributor

phyllisstein commented Dec 4, 2017

Done and done. Let me know if you have any other suggestions!

@leroix leroix merged commit b1dcef9 into atom:master Dec 5, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment