Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Only show lockfile warning on lazy activation #971

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

vinistock
Copy link
Member

Motivation

There are two types of workspace activation: eager or lazy.

Eager happens when we activate the extension. We check to see if there's a lockfile in the workspace root and then proceed to activate. Lazy happens when a Ruby document is opened in a workspace we had not activated before.

Currently, we're showing the lockfile warning on both, which is a poor experience. We should really only show it on lazy activations, giving the chance to the user to do something about it.

But if the workspace is not a Ruby workspace, then we shouldn't show it and should just default to not starting the LSP.

Implementation

This PR makes a few tweaks to the experience:

  • We now differentiate between an eager or a lazy activation
    • On eager activations, we do nothing if there's no lockfile (for the scenario of workspaces that may be purely written in another language)
    • On lazy activations, we still launch the language server, but we display a warning to the user prompting them to either look at the documentation or ignore that warning

@vinistock vinistock added the bugfix This PR will fix an existing bug label Jan 9, 2024
@vinistock vinistock self-assigned this Jan 9, 2024
@vinistock vinistock requested a review from a team as a code owner January 9, 2024 19:24
@parsley42
Copy link

This looks great to me and will definitely fix the UX.

@vinistock
Copy link
Member Author

Thanks @parsley42! I appreciate the feedback 🙏.

@vinistock vinistock merged commit 5e93ec0 into main Jan 10, 2024
10 checks passed
@vinistock vinistock deleted the vs/treat_eager_and_lazy_activation_differently branch January 10, 2024 16:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bugfix This PR will fix an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants