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

Reenable file path watcher, watching interpreter paths #1306

Merged
merged 20 commits into from
Jul 15, 2019

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Jul 10, 2019

Fixes #562.
Fixes #1216.
Updates #1076.

This moves the file path watching code down from LanguageServer into Server. It then has access to the interpreter's MainResolution.InterpreterPaths, and can use that list to watch instead of the (wrong) searchPath list from the initialization message.

Two conditions where a paths watcher can be created:

  • The paths watcher is created at the first didConfigurationChange event. Because the interpreter paths are populated after ReloadModule finishes, the interpreter paths are present (as a configuration change can only occur after the initialization is complete).
  • If the paths watcher detects a change, it calls ReloadModule, then afterwards will check the paths again and recreate itself if the paths have changed.

In both cases, the paths are accessed after ReloadAsync, so are in the correct state.

I need to do a bit more testing, but it should be equivalent to the way it was before I changed the searchPath generating code in the extension to not send everything at once.

@jakebailey jakebailey changed the title [WIP] Fix path watcher Fix path watcher Jul 12, 2019
@jakebailey
Copy link
Member Author

I think this is working. I need to go and remove a bunch of files from the table, as package uninstalls can mean that they no longer exist; keeping them there causes them to be reanalyzed, which causes weird things to happen in the dependency resolution.

@MikhailArkhipov I added a removal function to the document table. I can do this another way if it makes sense to be different.

@AlexanderSher One downside of removing documents is that if a user has opened a library file, then I won't be able to remove it (otherwise future calls can break, as another didOpen won't be issued), which means that the dependency resolver tries to deal with it and eventually prints something about missing keys. Is this an issue, or is it alright to ignore these? If you uninstall, then reinstall, things still work as expected, it's just slightly noisy.

@jakebailey jakebailey changed the title Fix path watcher Reenable file path watcher, watching interpreter paths Jul 12, 2019
@AlexanderSher
Copy link
Contributor

Have you tried to put breakpoint to the PythonAnalyzer.RemoveAnalysis to check if it is called?

@jakebailey
Copy link
Member Author

If the file has been opened by the user, then I must assume that it has been modified (I have no way of tracking that it was dirty, unless I'm sorely mistaken), so I do not remove it. This includes goto def to some library where the user leaves it open then removes the package (rare).

@jakebailey
Copy link
Member Author

Tested working in Linux after fixing #1324. Moving on to macOS now.

@jakebailey
Copy link
Member Author

File watching also works in macOS (though I did find #1326, which explains some of the weird paths I've seen in the past).

@jakebailey
Copy link
Member Author

@MikhailArkhipov I moved the reloading code into the RDT itself. This actually avoids any locking/unlocking, since it's purely removing stuff from the dictionary.

@jakebailey jakebailey merged commit fc77d42 into microsoft:master Jul 15, 2019
@jakebailey jakebailey deleted the fix-path-watcher branch July 15, 2019 22:16
jakebailey added a commit to jakebailey/python-language-server that referenced this pull request Nov 1, 2019
* first working path classfier, breaks existing path code for now

* inherit Comparer instead of implementing both

* don't do path depth comparison, preserve user ordering as before

* switch to using new path classification code in main module resolution

* fix typo

* clean up comment about paths in Server.cs

* use String.Split instead of regex when parsing script output

* test ordering of user provided paths

* add new normalize and trim helper, check preconditions as debug asserts rather than commenting on them

* Bring back search path watcher for interpreter paths

* make watchSearchPaths change code more obvious, remove stray semicolon

* perform check only once

* filter files to python-ish ones, up buffer size to maximum

* avoid reanalysis of non-existent files on reload by removing all unopened files from the table

* fix build

* check parsed path instead of unparsed line

* move RDT reloading into RDT
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.

Fix path watcher to watch only library paths IntelliSense/completions not updating after installing package into current virtual environment
4 participants