-
Notifications
You must be signed in to change notification settings - Fork 67
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
feature: code completion #503
feature: code completion #503
Conversation
src/render.js
Outdated
name: "stderr", | ||
text: `Failed to execute. ${error} Please refresh the page.`, | ||
}); | ||
$cell.find("div.thebelab-busy").css("visibility", "hidden"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code is repeated in the execute catch block, would be good to extract to a function and use in both places e.g. clearOnError(error)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/render.js
Outdated
if (!kernel) { | ||
console.debug("No kernel connected"); | ||
setOutputText(); | ||
events.trigger("request-kernel"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the only way to enable this feature is to have the kernel running, so it's reasonable to request it if its not there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the feedback!
right now it's doing the same thing the execute
function is doing, how should I change it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think leave it as-is 👍
@twyair I just tried this out in local dev and it work's great! One suggestion if you're able to extend this PR is to mention this in the docs - perhaps adding to a section to |
Brought this in! Thank @twyair I'll add the docs on the main branch |
No description provided.