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

add generic marker sequence and markers/decorations for problems #152671

Merged
merged 31 commits into from Jun 23, 2022
Merged

add generic marker sequence and markers/decorations for problems #152671

merged 31 commits into from Jun 23, 2022

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Jun 20, 2022 •
edited

This PR fixes #151769 and fixes #152391

@VSCodeTriageBot VSCodeTriageBot added this to the June 2022 milestone Jun 20, 2022
@meganrogge meganrogge marked this pull request as draft June 20, 2022 19:49
@meganrogge meganrogge marked this pull request as ready for review June 20, 2022 20:43
@meganrogge meganrogge marked this pull request as draft June 20, 2022 22:11
@meganrogge
Copy link
Contributor Author

meganrogge commented Jun 22, 2022 •
edited

working well except for the first run of the task

@Tyriar
Copy link
Member

Tyriar commented Jun 23, 2022

[4:09:59 PM] File change detected. Starting incremental compilation... matches but [4:07:41 PM] Starting compilation in watch mode... does not

Fixed in #152966, #152967

@Tyriar
Copy link
Member

Tyriar commented Jun 23, 2022

It looks a little off how the decoration shows up before any problems do:

image

Not sure there's a nice way to workaround that if we want to use the SetMark sequence.

@Tyriar
Copy link
Member

Tyriar commented Jun 23, 2022

It ended up in the wrong spot on my first test of the https://github.com/lunapaint/tga-codec build task:

image

@Tyriar
Copy link
Member

Tyriar commented Jun 23, 2022

Weird things happen on resize in Windows, probably because of reprinting:

Recording 2022-06-23 at 05 01 33

Above is also 2 decorations on top of one another.

Tyriar
Tyriar requested changes Jun 23, 2022
View reviewed changes
src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts Outdated Show resolved Hide resolved
src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts Outdated Show resolved Hide resolved
src/vs/platform/terminal/common/terminalProcess.ts Outdated Show resolved Hide resolved
src/vs/platform/terminal/common/capabilities/commandDetectionCapability.ts Show resolved Hide resolved
src/vs/platform/terminal/common/capabilities/commandDetectionCapability.ts Outdated Show resolved Hide resolved
src/vs/platform/terminal/common/terminalProcess.ts Outdated Show resolved Hide resolved
src/vs/workbench/contrib/terminal/browser/terminalEscapSequences.ts Outdated Show resolved Hide resolved
src/vs/workbench/contrib/terminal/browser/xterm/decorationAddon.ts Outdated Show resolved Hide resolved
src/vs/workbench/contrib/terminal/browser/xterm/decorationAddon.ts Show resolved Hide resolved
meganrogge and others added 3 commits June 23, 2022 09:44
@meganrogge @Tyriar
Update src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts
b82fe91 
Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
@meganrogge @Tyriar
Update src/vs/workbench/contrib/terminal/browser/xterm/decorationAddo…
7a0271e 
…n.ts

Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
@meganrogge
add e to file name
be9e602
@Tyriar Tyriar mentioned this pull request Jun 23, 2022
Closed
@meganrogge
Copy link
Contributor Author

It looks a little off how the decoration shows up before any problems do:

image

Not sure there's a nice way to workaround that if we want to use the SetMark sequence.

we could add a delay before showing it?

@meganrogge
Copy link
Contributor Author

meganrogge commented Jun 23, 2022 •
edited

It ended up in the wrong spot on my first test of the https://github.com/lunapaint/tga-codec build task:

image

It ended up in the wrong spot on my first test of the https://github.com/lunapaint/tga-codec build task:

did that happen on first compilation?

yep that's an issue i'm aware of and reproes w any task for the first compilation - doesn't match here for xterm.js. likely the case for luna paint as well
Screen Shot 2022-06-23 at 10 46 47 AM
so is an issue with the $tsc-watch problem matcher
Screen Shot 2022-06-23 at 10 48 47 AM

Tyriar
Tyriar previously approved these changes Jun 23, 2022
View reviewed changes
mjbvz
mjbvz previously approved these changes Jun 23, 2022
View reviewed changes
@meganrogge
h
c10717a 
Merge branch 'main' into merogge/problem-marker
@meganrogge meganrogge dismissed stale reviews from mjbvz and Tyriar via c10717a June 23, 2022 22:26
@meganrogge meganrogge enabled auto-merge (squash) June 23, 2022 22:29
@meganrogge meganrogge merged commit bcfdcf5 into main Jun 23, 2022
@meganrogge meganrogge deleted the merogge/problem-marker branch June 23, 2022 22:36
@github-actions github-actions bot locked and limited conversation to collaborators Aug 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Tyriar Tyriar Tyriar approved these changes

@mjbvz mjbvz mjbvz left review comments

Assignees

@meganrogge meganrogge

Labels
None yet
Projects
None yet
Milestone
June 2022
4 participants
@meganrogge @Tyriar @mjbvz @VSCodeTriageBot