Fix some edge cases in shell integration conpty handling #145346
Conversation
This starts to listen for cursor moves on the next frame to avoid listening when the cursor is on the wrong line (if conpty renders the sequence in the wrong spot). There is also some handling for when that handler doesn't trigger before command finished does. Fixes #143894
| // HACK: Fire command started on the following frame on Windows to allow the cursor | ||
| // position to update as conpty often prints the sequence on a different line to the | ||
| // actual line the command started on. | ||
| timeout(0).then(() => { | ||
| if (!this._currentCommand.commandExecutedMarker) { |
There was a problem hiding this comment.
if conpty does fire command started for the actual line it was started on, are there any consequences of this happening again?
I think we dispose of the placeholder decoration every time register decoration is called, which should prevent duplicates
There was a problem hiding this comment.
We only want to listen to cursor moves between start and executed, doing it outside that would be a waste as we wouldn't use it.
| if (this._commandMarkers.length === 0) { | ||
| // If the command start timeout doesn't happen before command finished, just use the | ||
| // current marker. | ||
| if (!this._currentCommand.commandStartMarker) { |
There was a problem hiding this comment.
since the timeout is 0, when will this happen?
There was a problem hiding this comment.
It may happen when running a command while another is running, I don't think it does in practice currently due to event loop related things in node-pty/conpty where data but it might and could in the future.
This starts to listen for cursor moves on the next frame to avoid listening when
the cursor is on the wrong line (if conpty renders the sequence in the wrong spot).
There is also some handling for when that handler doesn't trigger before command
finished does.
Fixes #143894
This fix seems pretty reliable, the only place I could break it is rarely this would happen for skipped commands:
Typed commands seemed to pretty much always work.