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

Handle when onDidEndTerminalShellExecution never fires #24123

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

anthonykim1
Copy link

@anthonykim1 anthonykim1 commented Sep 17, 2024

Related: #24078
We should rather have exitCode as a promise instead of the entire object as a promise for case:

  • onDidEndTerminalShellExecution never fires because because the python command never finishes.
    @ The case of where shell integration is enabled in zsh/pwsh in Windows but not inside Python REPL so python command won't finish until user exit()

Cases we need to handle:

@ Shell integration in zsh/pwsh works, but not python for windows

  1. executeCommand never actually runs the command because the python command never finishes
  2. onDidEndTerminalShellExecution never fires because because the python command never finishes

This PR takes care of the second case scenario.

@anthonykim1 anthonykim1 self-assigned this Sep 17, 2024
@anthonykim1 anthonykim1 changed the title Improvements to executeCommand to prevent forever waiting Handle when onDidEndTerminalShellExecution never fires Sep 18, 2024
@anthonykim1 anthonykim1 added feature-request Request for new features or functionality area-terminal labels Sep 18, 2024
@anthonykim1
Copy link
Author

anthonykim1 commented Sep 18, 2024

I've tested this on windows and it does not seem like we get stuck in a infinite loop, regardless of this PR.

I'm not sure if this is because

if (terminal.shellIntegration) {

is smart enough to realize subshell (python shell inside pwsh for example), does not support shell integration (for windows python in terminal in our scenario)

OR

because we aren't awaiting this exitCode hence we are never stuck in the infinite loop, since executeCommand will just return even when it is not sending any command to terminal.

#24138 should help what is happening on windows in the pwsh => python scenario.

@anthonykim1 anthonykim1 added the skip tests Updates to tests unnecessary label Sep 18, 2024
anthonykim1 added a commit that referenced this pull request Sep 19, 2024
Need logging to further investigate
#24123 (comment)
for windows.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-terminal feature-request Request for new features or functionality skip tests Updates to tests unnecessary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant