Skip to content

fix: color hints may display twice(#175476)#186926

Merged
hediet merged 2 commits into
microsoft:mainfrom
yshaojun:fix/175476
Jul 17, 2023
Merged

fix: color hints may display twice(#175476)#186926
hediet merged 2 commits into
microsoft:mainfrom
yshaojun:fix/175476

Conversation

@yshaojun

@yshaojun yshaojun commented Jul 3, 2023

Copy link
Copy Markdown
Contributor

fix #175476

@hediet hediet requested a review from aiday-mar July 4, 2023 08:39
@hediet hediet assigned aiday-mar and unassigned hediet Jul 4, 2023

@aiday-mar aiday-mar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi thanks for the work, after a second look I notice that the unit tests are failing. The end offset is not calculated correctly according to these tests. Some more work will need to be done to find a fix. Unfortunately we can not merge it with the unit tests failing.

@yshaojun

Copy link
Copy Markdown
Contributor Author

Hi thanks for the work, after a second look I notice that the unit tests are failing. The end offset is not calculated correctly according to these tests. Some more work will need to be done to find a fix. Unfortunately we can not merge it with the unit tests failing.

Hi Aiday, I checked the failing test case, found the token endIndex is 30, it's also the last column of a line. I think inline decoration's endOffset shouldn't be greater than line's last column, should we update the failing test case? Please check.

WX20230715-110245@2x

@aiday-mar

Copy link
Copy Markdown
Contributor

Hi thanks for the reply. @hediet I noticed you made some changes previously on the test code that this PR aims to change. If it wasn't you who wrote that specific test let me know. Do you think it would be okay to make the change proposed by @yshaojun?

@zvailanu98 zvailanu98 mentioned this pull request Jul 17, 2023
@hediet

hediet commented Jul 17, 2023

Copy link
Copy Markdown
Member

@yshaojun thanks for fixing this!

@hediet hediet enabled auto-merge July 17, 2023 12:47
@hediet hediet dismissed aiday-mar’s stale review July 17, 2023 12:48

superseded by hediets review

@hediet hediet merged commit b2c6bfb into microsoft:main Jul 17, 2023
@hediet hediet added this to the July 2023 milestone Jul 17, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Outdated] Color hints may display twice at view edges with line wrap

3 participants