Unify ruby-lsp-doctor with ruby-lsp command #2310
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
andyw8
force-pushed
the
andyw8/unify-ruby-lsp-doctor
branch
from
ea164ca to
6fb9a8e
Compare
andyw8
commented
Jul 15, 2024
| @@ -36,6 +36,10 @@ parser = OptionParser.new do |opts| | |||
| options[:experimental] = true | |||
| end | |||
|
|
|||
| opts.on("--doctor", "Run troubleshooting steps") do | |||
There was a problem hiding this comment.
I didn't add a -d short prefix since it's not something that would be run often, and also to prevent unwanted clashes with other options we may want to add in future.
andyw8
commented
Jul 15, 2024
| @@ -107,4 +111,25 @@ if options[:time_index] | |||
| return | |||
| end | |||
|
|
|||
| if options[:doctor] | |||
| if File.exist?(".index.yml") | |||
There was a problem hiding this comment.
Copied direct from exe/ruby-lsp-doctor.
andyw8
added
enhancement
andyw8
force-pushed
the
andyw8/unify-ruby-lsp-doctor
branch
from
6fb9a8e to
8e221d4
Compare
vinistock
approved these changes
Jul 15, 2024
exe/ruby-lsp-doctor
Outdated
| puts "indexing: #{indexable.full_path}" | ||
| index.index_single(indexable) | ||
| end | ||
| puts "The ruby-lsp-doctor command is deprecated. Please run `ruby-lsp --doctor` instead." |
There was a problem hiding this comment.
I vote for getting rid of this file. It's just used for troubleshooting anyway, it's not going to be super disruptive for users.
Let's just ensure it's not referenced in any docs or instructions.
There was a problem hiding this comment.
Any strong opinion @st0012 ?
There was a problem hiding this comment.
Yeah let's remove it 👍 Remember to update gemspec too.
andyw8
force-pushed
the
andyw8/unify-ruby-lsp-doctor
branch
from
8e221d4 to
3d38c69
Compare
This was referenced Jul 15, 2024
|
I've added a note to some recent issues to point out the new name. |
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.
Motivation
Improve discoverability of the troubleshooting tool.
Partially addresses #2296
Implementation
I chose to deprecate rather than fully remove since we have a number of issues mentioning the tool. We can revisit in future and fully remove.
Automated Tests
None
Manual Tests
./exe/ruby-lsp --doctorand verify the checks are performed../exe/ruby-lsp-doctorand verify it prints a deprecation notice.