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

Move command invalidation from decoration addon to command detection capability #145893

Closed
Tyriar opened this issue Mar 23, 2022 · 1 comment · Fixed by #146563 or #147116
Closed

Move command invalidation from decoration addon to command detection capability #145893

Tyriar opened this issue Mar 23, 2022 · 1 comment · Fixed by #146563 or #147116
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-persistence Relating to process reconnection or process revive terminal-shell-integration Shell integration, command decorations, etc. verified Verification succeeded
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Mar 23, 2022

We currently have this which runs on the renderer process only:

this._commandFinishedListener = capability.onCommandFinished(command => {
if (command.command.trim().toLowerCase() === 'clear' || command.command.trim().toLowerCase() === 'cls') {
this.clearDecorations();
return;
}
this.registerCommandDecoration(command);
});

What we should do instead to ensure these changes carry over across reconnects is to mark commands as invalid or something and fire an event inside the command detection capability. The decorations addon would then listen to it and react accordingly.

@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug terminal-shell-integration Shell integration, command decorations, etc. labels Mar 23, 2022
@Tyriar Tyriar added this to the April 2022 milestone Mar 23, 2022
@Tyriar Tyriar added the terminal-persistence Relating to process reconnection or process revive label Mar 25, 2022
@Tyriar
Copy link
Member Author

Tyriar commented Mar 25, 2022

Additionally, using clear and cls here will be inherently flaky, running cls in bash for example will do nothing.

A similar workaround was added to PartialCommandDetectionCapability yesterday which checks which erase in display was used:

this._terminal.parser.registerCsiHandler({ final: 'J' }, params => {
if (params.length >= 1 && (params[0] === 2 || params[0] === 3)) {
this._clearCommandsInViewport();
}
// We don't want to override xterm.js' default behavior, just augment it
return false;
});

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders terminal-persistence Relating to process reconnection or process revive terminal-shell-integration Shell integration, command decorations, etc. verified Verification succeeded
Projects
None yet
3 participants
@Tyriar @meganrogge and others