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

Add CommManager.comms shim #751

Merged
merged 3 commits into from
Sep 23, 2016

Conversation

philippjfr
Copy link
Contributor

@philippjfr philippjfr commented Sep 4, 2016

The current jupyter-js-services shim for the CommManager, which gets used by project's like the dashboard_server does not mirror the API of the real CommManager completely, which is generally no problem but I was hoping to at least expose a means to look up existing comms. bokeh 0.12.2 will use this part of the comm_manager API to register an on_msg callback on an existing Comm. In future bokeh will probably correctly set up a notebook extension and/or display plots as a widget so this should no longer be needed but for the time being it would be great if we could expose the comms on the shim. Tested with bokeh 0.12.2rc2 and the current dashboard_server master.

@SylvainCorlay
Copy link
Member

ping @jasongrout

@@ -58,9 +59,10 @@ namespace shims {
* comm is made. Signature of f(comm, msg).
*/
register_target (target_name, f) {
var handle = this.jsServicesKernel.registerCommTarget(target_name, function(jsServicesComm, msg) {
var handle = this.jsServicesKernel.registerCommTarget(target_name, _.bind(function(jsServicesComm, msg) {
Copy link
Member

Choose a reason for hiding this comment

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

You can use an arrow function here to automatically bind this:

(jsServicesComm, msg) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will do.

@philippjfr
Copy link
Contributor Author

@jasongrout I made the change you suggested. Do you have any other objections? Otherwise this is ready to merge I think.

@minrk minrk merged commit 387ba91 into jupyter-widgets:master Sep 23, 2016
@minrk minrk added this to the 6.0 milestone Sep 23, 2016
@github-actions github-actions bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 21, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants