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

Remove initialisation lock #894

Merged
merged 21 commits into from
Mar 22, 2020
Merged

Remove initialisation lock #894

merged 21 commits into from
Mar 22, 2020

Conversation

tomv564
Copy link
Contributor

@tomv564 tomv564 commented Feb 28, 2020

This PR proposes to replace the initialisation lock with necessary checks and state management.
As far as I know there is no requirement for servers to respond to initialise in a certain time, so we should stop punishing our users for using servers like Metals.

Will not attempt to fix other edge cases related to server initialization as they can be solved in future work.

Bugs to fix

  • Go to Definition by LSP seems not to utilize LSP properly for the new tab #845

    • Make sure sessions with same but different case paths match files being opened
    • Stop opening files outside workspace if workspace folders are defined
    • Make sure to not open more files for sessions that are initializing
  • 0.9.3 reintroduced issue #79 #860

    • REVERTING Disable a server only for a specific folder of the project, if it fails to init
      • We don't need complexity to support working with broken setups/servers
    • Do prefer longest workspace path
      • TODO: I didn't check what VSCode does with nested folders but we may want to match their behaviour.

Issues reintroduced

Besides locking logic, this was also reverted

To do

  • restore any valuable tests reverted

@tomv564
Copy link
Contributor Author

tomv564 commented Mar 10, 2020

Reviewed the old tests and decided to add a specific one for the open-in-second-folder-while-first-folder-starting-session case.

@tomv564 tomv564 changed the title WIP: Remove initialisation lock Remove initialisation lock Mar 14, 2020
@tomv564 tomv564 requested a review from rwols March 14, 2020 20:20
plugin/core/sessions.py Show resolved Hide resolved
plugin/core/windows.py Show resolved Hide resolved
plugin/core/windows.py Show resolved Hide resolved
plugin/core/windows.py Show resolved Hide resolved
tests/setup.py Outdated Show resolved Hide resolved
@tomv564
Copy link
Contributor Author

tomv564 commented Mar 16, 2020

Thanks for the review, you pointed out some stuff that is worth fixing and I'll take a moment to prepare for handling the flow server issue as well

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.

4 participants