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

Improve non-ruby workspace folder UX #968

Closed
wants to merge 1 commit into from

Conversation

parsley42
Copy link

This PR updates the "no lock file" warning UX to what I would expect given the original comments. Previously "Don't show again" still resulted in starting a ruby-lsp server for the folder even if there was no lock file. Now if there's no lockfile:

  • If disableMultirootLockfileWarning is true, it just skips starting the workspace, otherwise it gives the warning
  • Yes sends the user to the docs and doesn't start the workspace
  • Don't show again doesn't start the workspacea and updates disableMultirootLockfileWarning
  • No allows the workspace to start even without a lockfile

Motivation

My standard workspace includes a ruby and non-ruby root. Prior to this PR, there was no way to tell the extension not to start ruby-lsp for the non-ruby root and not to ask again.

Implementation

I tried to match the behavior of activateWorkspace to the original comment:
If one of the workspaces does not contain a lockfile, then we don't try to start a language server. Especially, clicking "Don't ask again" doesn't start the ruby-lsp server for the directory and doesn't give a warning.

Automated Tests

I honesn't don't know how to write the tests - I'm a devops engineer, not a javascript programmer, and it took me a long time to write this as-is.

Manual Tests

I built and installed my version of the plugin, and now all the possible responses to the "no lockfile" warning behave as expected - importantly, I say "don't ask again" and the ruby-lsp server won't start for the given directory.

Final Thoughts

I kind of expect I'm missing something here, and am preparing myself for a major "D'oh!" moment.

This matches the behavior of activateWorkspace to the comments.
Previously "Yes" and "Don't show again" still resulted in starting
a ruby-lsp server for the folder even if there was no lock file.
Now if there's no lockfile:
* If disableMultirootLockfileWarning is true, it just skips starting
  the workspace, otherwise it gives the warning
* Yes sends the user to the docs and doesn't start the workspace
* Don't show again doesn't start the workspacea and updates
  disableMultirootLockfileWarning
* No allows the workspace to start even without a lockfile
@parsley42 parsley42 requested a review from a team as a code owner January 8, 2024 19:25
@vinistock
Copy link
Member

Thank you for the contribution and the feedback on the experience. You're right, it's odd right now.

I was trying to create a picture in my head of the different scenarios and ended up creating a PR to differentiate eager and lazy activation, which I believe fixes this issue #971.

Can you let me know what you think about it? Does it cover the scenario you brought up?

@parsley42
Copy link
Author

Can you let me know what you think about it? Does it cover the scenario you brought up?

Yup, your PR is definitely UX++ over this one. I'll close, thanks.

@parsley42 parsley42 closed this Jan 9, 2024
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.

2 participants