Introduce option to prevent underlining multiple lines#1528
Introduce option to prevent underlining multiple lines#1528dbaeumer merged 7 commits intomicrosoft:mainfrom
Conversation
|
I think the video is a good example why underlining the first line is not a good idea. The error is about the fact that |
|
@dbaeumer That's a tradeoff that the user should be able to make on their own |
server/src/eslint.ts
Outdated
| } | ||
| } | ||
|
|
||
| const documentsByProblem = new WeakMap<ESLintProblem, TextDocument>(); |
There was a problem hiding this comment.
Why do we need this. Wouldn't it be good enough to pass the document to Diagnostics.create
| line: startLine, | ||
| character: 0, | ||
| }, | ||
| end: { |
There was a problem hiding this comment.
Shouldn't this be end: { line: startLine, character: uinteger.MAX_VALUE }
server/src/eslint.ts
Outdated
| }, | ||
| end: { | ||
| line: startLine, | ||
| character: Number.MAX_SAFE_INTEGER, |
There was a problem hiding this comment.
This should be uinteger.MAX_VALUE do to the restrictions we have in LSP.
There was a problem hiding this comment.
Oops, didn't know that was a thing :)
dbaeumer
left a comment
There was a problem hiding this comment.
See comment about uinteger.MAX_VALUE
|
The changes look good to me. @aleclarson you need to accept the CLA (see #1528 (comment)). I will not be able to merge this without you accepting it. |
|
@microsoft-github-policy-service agree |
I rebased #463 in hopes that you'll reconsider merging this useful feature.
Your main concern seems to be consistency, but that's not a concern shared by ESLint maintainers:
vscode-eslint-singleLineUnderline.mov