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.

Sign up for GitHub

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

Jump to bottom

Harden shell integration handlers in renderer to handle multiple start sequences (A, B) properly #151228

Closed
Tyriar opened this issue Jun 3, 2022 · 1 comment · Fixed by #152136
Closed

Harden shell integration handlers in renderer to handle multiple start sequences (A, B) properly #151228

Tyriar opened this issue Jun 3, 2022 · 1 comment · Fixed by #152136
Assignees
@Tyriar
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-shell-integration Shell integration infrastructure, command decorations, etc. verified Verification succeeded
Milestone
June 2022

Comments

@Tyriar
Copy link
Member

Tyriar commented Jun 3, 2022

A reason the smoke tests were failing was because bash plain PS1 prompt support was lacking #150478

What ended up happening is the PS1/PS2 would continually get wrapped in our sequences so multiple A and B shell integration sequences would come through, the renderer handled that by creating a new decoration for each one. We should harden the shell integration code to make sure we handle this case by using the latest sequence.
@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug terminal-shell-integration Shell integration infrastructure, command decorations, etc. labels Jun 3, 2022
@Tyriar Tyriar added this to the June 2022 milestone Jun 3, 2022
This was referenced Jun 9, 2022
Closed
Closed
Tyriar added a commit that referenced this issue Jun 15, 2022
@Tyriar
Try prevent duplicate placeholder decorations showing
711dc5a 
This adds these protections:

- Placeholders are now always disposed of before a new one is created,
this was the main reason for these issues.
- Prevent the command started event firing if it has already fired on
the same line.

Fixes #151228
Tyriar added a commit that referenced this issue Jun 15, 2022
@Tyriar
Try prevent duplicate placeholder decorations showing
adf9340 
This adds these protections:

- Placeholders are now always disposed of before a new one is created,
this was the main reason for these issues.
- Prevent the command started event firing if it has already fired on
the same line.

Fixes #151228
@Tyriar Tyriar mentioned this issue Jun 15, 2022
Merged
@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jun 15, 2022
justschen pushed a commit to justschen/vscode that referenced this issue Jun 16, 2022
@Tyriar
Try prevent duplicate placeholder decorations showing
537b06d 
This adds these protections:

- Placeholders are now always disposed of before a new one is created,
this was the main reason for these issues.
- Prevent the command started event firing if it has already fired on
the same line.

Fixes microsoft#151228
@Tyriar
Copy link
Member Author

Tyriar commented Jun 29, 2022

Marking as verified as I haven't seen any flakiness recently

@Tyriar Tyriar added the verified Verification succeeded label Jun 29, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Tyriar Tyriar

Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-shell-integration Shell integration infrastructure, command decorations, etc. verified Verification succeeded
Projects
None yet
Milestone
June 2022
Development

Successfully merging a pull request may close this issue.

Try prevent duplicate placeholder decorations showing microsoft/vscode
3 participants
@Tyriar @meganrogge @vscodenpa