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

Refactor keyboard handling #5076

Merged
merged 6 commits into from Mar 3, 2014
Merged

Refactor keyboard handling #5076

merged 6 commits into from Mar 3, 2014

Conversation

ellisonbg
Copy link
Member

Fixes #4886

This creates a new JS module as base/keyboard.js exposed as IPython.keyboard for all of our keyboard handling. This is meant to replace the keyboard related stuff in IPython.utils. But the removal of the old stuff will come later once we have better tests in place. This should wait for #5053 so tests of keyboard.js can be added.

@ellisonbg ellisonbg added this to the 2.0 milestone Feb 8, 2014
@ellisonbg ellisonbg self-assigned this Feb 8, 2014
@takluyver
Copy link
Member

@ellisonbg I see #5053 has gone in, and you have added some tests for keyboard.js. Do you want to add some more now, or is this ready for review?

@ellisonbg
Copy link
Member Author

This is ready for review. We can add more tests over time, but this is a
good start for the core of it.

On Fri, Feb 14, 2014 at 11:57 AM, Thomas Kluyver
notifications@github.comwrote:

@ellisonbg https://github.com/ellisonbg I see #5053https://github.com/ipython/ipython/pull/5053has gone in, and you have added some tests for keyboard.js. Do you want to
add some more now, or is this ready for review?

Reply to this email directly or view it on GitHubhttps://github.com//pull/5076#issuecomment-35118813
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@minrk
Copy link
Member

minrk commented Feb 15, 2014

This is ready for review.

Did you want to get the tests passing, first?

@ellisonbg
Copy link
Member Author

Hmm, that is odd, this shouldn't have changed anything that would affect
existing tests, but I will look into it (I guess this is why we have test :)

On Fri, Feb 14, 2014 at 10:36 PM, Min RK notifications@github.com wrote:

This is ready for review.

Did you want to get the tests passing, first?

Reply to this email directly or view it on GitHubhttps://github.com//pull/5076#issuecomment-35148851
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@ellisonbg
Copy link
Member Author

OK, rebased, tests passing and ready for final review.

@minrk
Copy link
Member

minrk commented Mar 3, 2014

I thought I already merged this one...Merging now.

minrk added a commit that referenced this pull request Mar 3, 2014
@minrk minrk merged commit b721408 into ipython:master Mar 3, 2014
@ellisonbg
Copy link
Member Author

Yes, I forgot about it myself - it needed rebasing. Thanks!

On Mon, Mar 3, 2014 at 2:54 PM, Min RK notifications@github.com wrote:

Merged #5076 #5076.

Reply to this email directly or view it on GitHubhttps://github.com//pull/5076
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@ellisonbg ellisonbg deleted the keyboard branch March 6, 2014 22:59
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor and consolidate different keyboard logic in JavaScript code
3 participants