Skip to content

Remove inlay hint in diff views #3388

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

Merged
merged 2 commits into from
Mar 3, 2020
Merged

Conversation

vbfox
Copy link
Contributor

@vbfox vbfox commented Mar 1, 2020

If the left side of a diff view that contain the old version of the file apply inlays they are misplaced and produce a weird display:

image

After the change:

image

The detection is done by blacklisting the url schemes used by git and subversion scm extensions, whitelisting file is also possible but neither is perfect as VSCode now support both pluggable scm extensions and pluggable remote filesystems. But I suspect that the list of scm extensions is more easily manageable.

Note: I can rebase on #3378 if needed as it touches the same lines of code

If the left side of a diff view that contain the old
version of the file apply inlays they are misplaced.

The detection is done by blacklisting the url schemes used
by git and subversion scm extensions.
@@ -90,7 +92,14 @@ class HintsUpdater {

private get allEditors(): vscode.TextEditor[] {
return vscode.window.visibleTextEditors.filter(
editor => editor.document.languageId === 'rust',
editor => {
if (editor.document.languageId !== 'rust') {

This comment was marked as resolved.

@vbfox
Copy link
Contributor Author

vbfox commented Mar 1, 2020

Reading a little bit more code I think i'll also change & share code with Ctx.activeRustEditor as all the callers assume that the document with the URI is it's current version in the LSP server but that's not the case for scm diff views so it'll cause the same class of problems.

@matklad
Copy link
Member

matklad commented Mar 2, 2020

Yeah, makes sense. I wonder if we should maybe also lift visibleRustEditors to Ctx.

Also move visibleRustEditors to Ctx
@vbfox
Copy link
Contributor Author

vbfox commented Mar 2, 2020

Done, I also made the check a separate function in utils and called it everywhere the languageId was checked.

@matklad
Copy link
Member

matklad commented Mar 3, 2020

bors r+

Thanks!

@bors
Copy link
Contributor

bors bot commented Mar 3, 2020

@bors bors bot merged commit b55d22e into rust-lang:master Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants