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.

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

Problem pane errors persist when file is deleted from filesystem #58088

Closed
aaroncraig10e opened this issue Sep 6, 2018 · 7 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone

Comments

@aaroncraig10e
Copy link

Issue Type: Bug

As suggested by @dbaeumer (#50448 (comment)), I'm opening a new issue regarding this behavior.

I noticed this when switching between branches.

branch-a has a file, index.tsx, which contains TS errors. Opening this file in the editor causes the errors to be reported in the Problems pane.

branch-b has no such file.

While still on branch-a, close index.tsx, notice that errors persist in the Problems pane.

Switch to branch-b. Notice the file is no longer present in the file browser window, however

  • issues persist in the Problem pane
  • The folders that comprise the path to the (now inexistant file) are displayed in the error state.

I can also replicate the issue with all extensions disabled.

VS Code version: Code - Insiders 1.27.0-insider (7b9afc4, 2018-09-03T09:07:33.052Z)
OS version: Darwin x64 17.7.0

System Info
Item Value
CPUs Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz (8 x 2900)
GPU Status 2d_canvas: enabled
checker_imaging: disabled_off
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: enabled
rasterization: enabled
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled
Load (avg) 3, 4, 3
Memory (System) 16.00GB (0.87GB free)
Process Argv /Applications/Visual Studio Code - Insiders.app/Contents/MacOS/Electron
Screen Reader no
VM 0%
Extensions (27)
Extension Author (truncated) Version
Bookmarks ale 9.0.3
project-manager ale 8.0.0
vscode-eslint dba 1.5.0
javascript-ejs-support Dig 0.3.2
githistory don 0.4.2
gitlens eam 8.5.6
tslint eg2 1.0.38
jenkins-declarative-support jmM 0.1.0
terraform mau 1.3.4
dotenv mik 1.0.1
python ms- 2018.8.0
atom-keybindings ms- 3.0.4
azure-account ms- 0.4.3
vsliveshare ms- 0.3.623
debugger-for-chrome msj 4.9.1
vscode-jest Ort 2.9.0
vscode-docker Pet 0.1.0
material-icon-theme PKi 3.5.3
ruby reb 0.20.0
java red 0.30.0
jenkinsfile-support sec 0.1.0
slack soz 0.0.14
code-spell-checker str 1.6.10
vscode-java-debug vsc 0.12.1
vscode-java-test vsc 0.8.0
vscode-react-native vsm 0.6.16
markdown-all-in-one yzh 1.6.0

(6 theme extensions excluded)

@vscodebot vscodebot bot added the insiders label Sep 6, 2018
@mjbvz mjbvz self-assigned this Sep 6, 2018
@aaroncraig10e
Copy link
Author

I should add that if the deleted file is still open in the editor (with the Deleted from disk message in the tab title), the Problem pane correctly removes errors for that file when you close it.

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 27, 2018

@bpasero Need advice on what to do here. TS doesn't get a close event if there is still a Deleted from disk editor open

@bpasero
Copy link
Member

bpasero commented Oct 27, 2018

@mjbvz this is a setting the user can change (workbench.editor.closeOnFileDelete) which we recently (2 months ago?) flipped to be disabled by default. If the file gets deleted outside of VSCode (or e.g. by a git operation) we keep it open and the model is fully alive. The user has to close the tab to release the model.

How can I help on this issue here?

@mjbvz
Copy link
Collaborator

mjbvz commented Feb 21, 2019

Related #61773 and #59363

@mjbvz
Copy link
Collaborator

mjbvz commented Feb 21, 2019

@dbaeumer This issue was caused by fixing #47386. I'm still having trouble understanding what I'm supposed to do for coordinating diagnostics between an extension and a task. Here's what I want to happen:

  • For files that are open in VS Code, TS Server is responsible for their diagnostics. Errors from any tsc task are not shown in the problems view
  • For files that are closed, the tsc task is responsible for their diagnostics.
  • When a user opens a previously closed file, the diagnostics in the problems view should switch from the tsc task diagnostics to the TS Server diagnostics.
  • When a user closes a previously open file, the diagnostics in the problem views should switch from the TS Server diagnostics (which should be deleted) to the tsc task diagnostics

I can't seem to implement that fourth point today with how diagnostics and tasks work. Is there any way to implement it with the current apis? I'm tempted to revert the fix for #47386 because I suspect more people are hitting this problem vs that one

/cc @alexr00

@dbaeumer
Copy link
Member

Having a TSC task to get project wide errors is IMO a workaround for the fact that at least in the past the TypeScript language server couldn't compute them efficiently. Running a full compiler and a language server IMO only adds unnecessary CPU and memory overhead.

@mjbvz do you know if the TypeScript language server made any progress in computing project wide errors and being able to emit JS efficiently. If this is the case leveraging this would IMO be the best options (see tsconfig compileOnSave https://www.typescriptlang.org/docs/handbook/tsconfig-json.html) and removes the confusion around having a task and a language server that produces errors.

The rule we used for removing diagnostics from the problem panel on document close was as follows so far:

  • for single file language server (e.g. linters) the error should be removed
  • for programming languages with a project context the corresponding extension should manage errors on project basis.

Regarding the forth point: this can only work for watch tasks (I started to implement this a while back but then didn't continue see branch dbaeumer/47386). Restoring errors from a non watch task might be very confusing to the user since it might just have fixed it by changing code in another file.

With the new custom task API (coming this month) the Typescript extension would be able to implement everything itself and manage all errors itself even the once generated by a task.

So I would propose to do the following steps:

  1. see if we can leverage the TS server more for project wide diagnostics and JS emit
  2. if above is not possible investigate how we best restore problems from watch tasks
  3. if this is unsatisfactory for users the TS extension implements custom tasks and manages / merges diagnostics itself.

@mjbvz
Copy link
Collaborator

mjbvz commented Feb 21, 2019

Thanks. Yes compile on save would be ideal but it sounds like it will require larger work. I'm syncing with TS again to see if the apis are ready for us to use and to try to develop a definite plan for adopting this. I should know more by Monday

Until then, I'm going to revert the fix for #47386 for two reasons: I believe it is less likely to be hit than this error, and all you have to do to get out of the bad state from #47386 is re-trigger the task, while there is no way to clear the ghost problems from this error

@mjbvz mjbvz closed this as completed in 553e6e4 Feb 21, 2019
sandy081 pushed a commit to vldmrkl/vscode that referenced this issue Feb 22, 2019
@octref octref added the verified Verification succeeded label Feb 27, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants