Fixes for Hydrogen compatibility

n-riesco / ijavascript

IJavascript is a javascript kernel for the Jupyter notebook

Other

2.19k stars 185 forks source link

Fixes for Hydrogen compatibility #29

Closed willwhitney closed 9 years ago

willwhitney commented 9 years ago

Added an error message if the signature is incorrect (previously it was just swallowed)
Eliminated the signature check if key = ""
- This was done before with (this.signature === ''), but it didn't actually work
Only respond with HTML if the content is really HTML
- response types like HTML are meant for progressive enhancement; they aren't required and shouldn't be used unless necessary

n-riesco commented 9 years ago

Thank you for the PR!

I was concerned about removing "text/html", but I've checked and it works with both IPython 1.2.1 and 3.1.0.

Please, would you update the AUTHORS file? I ask all the contributors to do so to confirm that: 1) they are the copyright holder of the contribution and 2) they agree for their contribution to be distributed within IJavascript under the terms in the LICENSE file.

n-riesco commented 9 years ago

One more thing... Please, would send the PR against https://github.com/n-riesco/jmp ? I'm in the process of refactoring IJavascript to use jmp.

rgbkrk commented 9 years ago

:wink:

willwhitney commented 9 years ago

OK, this should be all set, and I opened https://github.com/n-riesco/jmp/pull/3 against jmp.

n-riesco commented 9 years ago

Hi Will,

would you update this PR so that I can merge it into IJavascript too? Since I've already updated IJavascript to use jmp, the only changes I need are:

add yourself to the AUTHORS file
remove "text/html": toHtml(result), from lib/sm_server.js

thx,

Nico

willwhitney commented 9 years ago

Yup, changed and should be good to merge now.