-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat: better setup diagnostics #436
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mhuisi
force-pushed
the
mhuisi/setup-diagnostics
branch
2 times, most recently
from
April 29, 2024 14:32
ccb8ab8
to
e73a0e7
Compare
I seem to have accidentally disabled this half a year ago and nobody noticed, so we might as well get rid of it for now.
mhuisi
force-pushed
the
mhuisi/setup-diagnostics
branch
2 times, most recently
from
April 30, 2024 15:34
daa86c0
to
9df1510
Compare
mhuisi
force-pushed
the
mhuisi/setup-diagnostics
branch
from
April 30, 2024 16:00
e9509dc
to
73b1363
Compare
Since the server startup swallows stdout and stderr output of the launched process, if `lake serve` goes wrong for whatever reason despite Lean being installed correctly, there will be no error. This ensures that we at least display an error if `lake --version` already goes wrong.
VS Code has a bug where even if you await the result of `window.withProgress`, the progress bar will still linger a bit afterwards. This means that when executing lots of quick commands with progress, irritatingly many progress bars briefly get stacked on top of one another. This ensures that quick running commands don't display a progress bar at all.
Some commands report errors on stdout, some on stderr. Now we just report all output.
Ensures that: - Notifications without input never block the extension - Notifications with optional input never block the extension - If a notification blocks the extension, it must be modal so that it cannot disappear and block the extension without the user noticing
mhuisi
added a commit
that referenced
this pull request
May 23, 2024
This PR adds additional diagnostics when creating and cloning projects, as both of these commands depend on various external dependencies and can be used before opening a Lean file for the first time (i.e. the global setup diagnostics of #436 do not catch issues at this point). It also refactors the architecture in #436 to make it easier to add new kinds of diagnostics. The following aspects of the user's setup are checked when creating projects: 1. Whether Curl and Git are installed (Error; reference to setup guide) 2. Whether Elan is installed and reasonably up-to-date (Error if not installed as we use `lean +toolchain` for new project, modal warning if out-of-date; Elan installation / Elan update offered) 3. Whether some version of Lean can be found (Error) 4. Whether the toolchain associated with the new project is a Lean 3 toolchain (Error) 5. Whether the toolchain associated with the new project is using a Lean version from before the first Lean 4 stable release (Modal warning) 6. Whether some version of Lake can be found (Error) The following aspects of the user's setup are checked when cloning projects: 1. Before cloning: Whether Curl and Git are installed (Error; reference to setup guide) 2. Whether Elan is installed and reasonably up-to-date (Modal warning; Elan installation / Elan update offered) 3. Whether some version of Lean can be found (Error) 4. Whether the toolchain associated with the new project is a Lean 3 toolchain (Error) 5. Whether the toolchain associated with the new project is using a Lean version from before the first Lean 4 stable release (Modal warning) 6. Whether some version of Lake can be found (Error)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR has three main goals:
Setup diagnostic warnings and errors
There are two separate kinds of setup diagnostics: Global- and project-level diagnostics.
If someone is willingly using a "weird" setup that triggers warning-level diagnostics, the corresponding notifications can be turned off using the
lean4.showSetupWarnings
configuration option.Setup diagnostic dump
In the forall menu, a new command "Troubleshooting: Show Setup Information" has been added that can be used to gather information about the user's system and their Lean setup. It includes the following information:
Clean-up of user-facing legacy features
lake serve
has been removed.Refactorings
LeanClientProvider
has been lifted out to the outer global-level setup diagnostics and theLeanClientProvider
is now much simpler than it used to be.LeanClientProvider
has been deleted. Callinglean --version
(rarely) a second time when that specific Lean toolchain has already been installed by a previous call should be sufficiently cheap that no cache should be needed.config.ts
has been cleaned up.window.show(Error|Warning|Information)...
have been refactored to use a safer API that ensures the following invariants to prevent the VS Code extension from being blocked on a transient notification:Affected issues
Fixes #323: We now have a general mechanism to improve these kinds of setup issues.
Closes #326: This setting has been removed.
Fixes #388: The refactored startup logic respects overrides.
Closes #400: This setting has been removed.
Closes #440: This setting has been removed. I checked with @eric-wieser to make sure that the setup exhibiting this issue can be implemented without this setting as well.