Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Diagnostic list flickers annoyingly when opening a document #512

Description

@erictraut

In VSCode, if you display all diagnostics for your project (i.e. set the "python.analysis.openFilesOnly" to "false"), you can view the list in the "Problems" panel. If you open a Python file that contains a reported diagnostic the analysis diagnostics for that file briefly disappear and then reappear. This is not only aesthetically unpleasing, it's also disorienting if you just clicked on one of these items to open the file and view an error in the editor.

The problem appears to be that the PLS is reporting diagnostics in two phases: parsing and analysis. When a file is closed, the server sees version -1 of the file, but when it's opened, the server sees version 1. This causes the server to re-parse and re-analyze the file from scratch. (As an aside, this seems wasteful given that the file hasn't changed. Maybe it could be skipped altogether?) When the parse is complete, the server reports the incomplete diagnostics, which don't contain the analysis drags. Then shortly later it reports the full diagnostics. In the meantime, VSCode has updated the diagnostic list in the "Problems" panel.

I don't fully understand the original reason behind updating the parse and analysis errors independently, so I'm not sure of the correct fix. My attempt at a fix is to comment out the following three lines from EditorFile.UpdateParseDiagnostics:

// if (diags != null) {
//     PublishDiagnostics(diags);
// }

This skips the reporting of the partial diagnostics and waits for completion of the analysis. With this change, the annoying flicker, and I haven't noticed any bad side effects.

Activity

  1. MikhailArkhipov commented on Dec 27, 2018

    @MikhailArkhipov

    The parse errors are produced fast and the separate publishing allows users to see syntax issues like missing braces faster as they type. Analysis can take literally tens of minutes to complete. Live syntax check was a very popular request since the only alternative is to run PyLint on save (which is what happens with Jedi completion engine).

    And yes, the issue with open/not open files is known, the code is somewhat legacy since it came from VS which is project based into the world where there is no project and just bunch of folders.

  2. MikhailArkhipov commented on Feb 9, 2019

    @MikhailArkhipov

    So, bottom line is that first reporting syntax errors and then combo of syntax + analysis is inevitable as this is how VS Code works. It has no API for incremental error list population like big VS has and the entire snapshot of diagnostics has to be sent each time. Delaying reporting until analysis completes deprives user from fast appearance of squiggles over syntax errors.

    What we can do in the new code base is throttle reporting of syntax errors a bit so if analysis completes within, say, 200-300 ms which is realistic in the new analysis engine, then only combo will be reported thus reducing flicker.

  3. erictraut commented on Feb 9, 2019

    @erictraut
    Author

    The problem here is not with VS Code but in the way you're reporting the errors. It's fine to report the errors in two passes — syntax and then analysis. That's how most compilers and language tools work. The problem is that when you run the pass a second time (due to a change in the file), you're discarding all of the previous errors and reporting only the syntax errors for the new version of the file.

    What you're doing now is this:

    1. Report syntax errors [version 1]
    2. Report syntax errors [version 1] + analysis errors [version 1]
    3. Report syntax errors [version 2]
      4.Report syntax [version 2] + analysis errors [version 2]

    What you should be doing is retaining the analysis errors from the previous pass until they are replaced by new analysis errors. Like this:

    1. Report syntax errors [version 1]
    2. Report syntax errors [version 1] + analysis errors [version 1]
    3. Report syntax errors [version 2] + analysis errors [version 1]
      4.Report syntax [version 2] + analysis errors [version 2]

    After step 3, there's a small chance the reported analysis errors will be out of date for a short period (until you get to step 4), but that's not a big deal. It's preferable to the current behavior. And most of the time the analysis errors won't change much (if at all) between steps 3 and 4, so the user won't notice any difference.

  4. MikhailArkhipov commented on Feb 10, 2019

    @MikhailArkhipov

    We could try that. However, openFilesOnly = true is the only option now. We no longer open and analyze all files in the workspace and the setting has no effect. Therefore difference most probably will be very small since new analysis engine adds very few messages at the moment, like unresolved imports.

  5. erictraut commented on Feb 10, 2019

    @erictraut
    Author

    Presumably, the analysis engine will add more diagnostics in the future. Even if you currently support only a few analysis errors at the moment, this change is still important because the annoying and disorienting error behavior will occur after an open file with analysis errors are edited.

  6. MikhailArkhipov commented on Feb 14, 2019

    @MikhailArkhipov

    So the method below produces odd presentation, may not be desirable. Say, I type import pand. it gets squiggled as unresolved. Then I continue typing but squiggle may remain for a several seconds until analysis finishes. I'd rather take error list update, it is easy to hide it. Persistent stale squiggle is annoying and there is no way to get rid of it.

  7. erictraut commented on Feb 14, 2019

    @erictraut
    Author

    I'm surprised to hear that analysis of a single file can take several seconds. It should be at the front of the priority queue in this case. I'd expect it to take tens of milliseconds, or hundreds at most.

    Even if it takes a couple of seconds, I still think it's preferable to retain the existing errors until they are replaced. The case you described is an edge case, and it's better to optimize the experience for the more common case.

  8. MikhailArkhipov commented on Feb 14, 2019

    @MikhailArkhipov

    When dependencies are loaded, yes. In the case above fixing import brings huge subsystem into the view. Its is analysis and analysis of all its dependencies may take 10-20 seconds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions