-
Notifications
You must be signed in to change notification settings - Fork 29.3k
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
Need for notebook pre-execute handler #212427
Comments
Additional idea/different form of API request: We could have "change event" which would get triggered every time user would type text in the text input box. This would allow Python extension to detect whether enter was typed in empty line or with text. If empty line, we can trigger submit and execute for the IW REPL. |
This sounds like a reasonable thing to just implement in core, without needing any extra API or contribution from extensions |
It's a text document so it's already available in the EH.
Can you elaborate why do we need to access user's command/code before execution? (what feature it is going to support) |
Right, @amunger have also made me aware of this. We discussed this yesterday but the problem about the text document "onDidChange"? event is how do we know if user is done typing and are ready to run their command. We could watch if user has pressed enter on a blank line like @karthiknadig mentioned to me.
However, I'm bit concerned that relying on constantly fired onDidChange event would be pretty expensive in terms of the number of request and response coming back and forth between Typescript side and Python side when user hasn't done typing their Python command and wants to type more into the text box. Meaning we could end up watching for every non-empty line case of user typing in their characters, which seems like a waste. The biggest concern and the reasoning behind asking for the "pre-execute" handler is wanting to make sure Python extension can receive latest information on user's Python command before actually running the command and returning the output of it for the purposes of: #212051
This is to ensure we can execute on "enter" Once enter is made the basic keybinding for executing commands in IW textbox and smartly determine to wait instead of executing if we can figure out something is incomplete command. @amunger I might be missing something so feel free to comment! |
I think this can be a core feature. If the input is empty, enter should not trigger an execution, just like what you get in other inputs, e.g., debug console, find widget. |
I would argue why do we prevent users from running the code they type in if they decide to? If we want to warn users about the code being "invalid/incomplete", diagnostics is probably the right way. |
Here is my understanding of the options discussed and their drawbacks
|
No longer needed as should be resolved with: microsoft/vscode-python#23442 |
Related: #212051
With the Python REPL IW experiment, Python extension needs access to text inside user's text input box in IW in order to determine whether they have typed complete or incomplete Python command. I assume this will be part of notebook API.
This needs to handle before the already existing execute-handler since by the time execute-handler is called is too late in a way that the cell output is already attached to the UI. We need to be able to access user's command before execute handler is called.
The text was updated successfully, but these errors were encountered: