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

band-aid for completion #5112

Merged
merged 1 commit into from
Feb 22, 2014
Merged

band-aid for completion #5112

merged 1 commit into from
Feb 22, 2014

Conversation

minrk
Copy link
Member

@minrk minrk commented Feb 13, 2014

Unlike @Carreau's attempt at using proper CodeMirror completion, this is just a band-aid to get the existing completion back to working for 2.0.

This adds a keypress handler on the completer, which then calls insert with the charCode of the event, replacing the final elif branch of the keydown handler.

This cannot be done with keydown, since keydown doesn't know what character is incoming, only the hardware key that is struck.

closes #4860

Unlike @Carreau's attempt at using proper CodeMirror completion,
this is just a band-aid to get the existing completion back to working for 2.0.

This adds a keypress handler on the completer,
which then calls insert with the charCode of the key press event,
replacing the final `elif` branch of the keydown handler.

This cannot be done with keydown,
since keydown doesn't know what character is incoming,
only the hardware key that is struck.
@minrk
Copy link
Member Author

minrk commented Feb 13, 2014

I am not super confident about this implementation, but it seems to work and I don't know how to write tests for it. If other folks could play with it and confirm that I didn't break everything, or at least that it's net less broken than master, that would be very helpful.

@jdfreder
Copy link
Member

Hey @minrk I'm trying to test this branch for you right now but I'm a little confused about what exactly this fixes. fr+TAB, from IPy+TAB, from IPython.ht+TAB, and from IPython.html import wid+TAB all seem to work in master (and this branch). In a following cell wid+TAB completes widgets successfully in both master and this branch. What am I supposed to be trying that doesn't work in master?

@takluyver
Copy link
Member

@jdfreder : I believe that typing while the completer is open currently doesn't work in master.

@jdfreder
Copy link
Member

@takluyver thanks for clarifying, that was it.
@minrk verified working here 👍

@jdfreder
Copy link
Member

Linked to #4860

@minrk minrk added this to the 2.0 milestone Feb 21, 2014

// if empty result return
if (!this.raw_result || !this.raw_result.length) return;

// When there is only one completion, use it directly.
if (this.autopick == true && this.raw_result.length == 1) {
if (this.autopick && this.raw_result.length == 1) {
Copy link
Member

Choose a reason for hiding this comment

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

=== 1

@ellisonbg
Copy link
Member

I am in favor of this approach to fixing the bug for 2.0. I have verified that it does fix the issue.

@ellisonbg
Copy link
Member

Code looks good, does anyone want to weigh in on this one? @Carreau @fperez

@fperez
Copy link
Member

fperez commented Feb 21, 2014

thanks for the heads-up, will test now.

@fperez
Copy link
Member

fperez commented Feb 22, 2014

Yup, confirming here that it does indeed improve things. Many thanks!! Merging now.

fperez added a commit that referenced this pull request Feb 22, 2014
Band-aid for completion in the notebook: not the ideal solution, but will have to do the job for now so that typing while completing works.

closes #4860
@fperez fperez merged commit e18be79 into ipython:master Feb 22, 2014
@minrk minrk deleted the completion-band-aid branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Band-aid for completion in the notebook: not the ideal solution, but will have to do the job for now so that typing while completing works.

closes ipython#4860
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.

Completer As-You-Type Broken
5 participants