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

Wrap BUNDLE_GEMFILE path in quotes #723

Closed
wants to merge 1 commit into from
Closed

Wrap BUNDLE_GEMFILE path in quotes #723

wants to merge 1 commit into from

Conversation

stilist
Copy link

@stilist stilist commented Jul 28, 2023

Motivation

I store some of my projects in iCloud, so paths start with ~/Library/Mobile Documents/com~apple~CloudDocs, which has a space. Without this change, getServerVersion fails with (for example)

Error restarting the server: Command failed: BUNDLE_GEMFILE=/Users/jordancole/Library/Mobile Documents/com~apple~CloudDocs/Projects/drjohndee.net/.ruby-lsp/Gemfile bundle exec ruby -e "require 'ruby-lsp'; print RubyLsp::VERSION"
/bin/sh: Documents/com~apple~CloudDocs/Projects/drjohndee.net/.ruby-lsp/Gemfile: No such file or directory

The error shows that this is parsed as running Documents/com~apple~CloudDocs/Projects/drjohndee.net/.ruby-lsp/Gemfile with BUNDLE_GEMFILE=/Users/jordancole/Library/Mobile as an environment variable.

Implementation

I searched for BUNDLE_GEMFILE and wrapped the assignment in double quotes. It’s simplistic, but I think it's correct.

Automated Tests

I haven't added tests because I'm not sure how to implement them.

Manual Tests

mkdir -p ~/"vscode-ruby-lsp path-test"
cd ~/"vscode-ruby-lsp path-test"
echo "puts 1" > test.rb
code .

Client.getServerVersion should run without error.

Allows the path to include spaces.
@stilist stilist requested a review from a team as a code owner July 28, 2023 02:31
@stilist stilist requested review from Morriar and egiurleo July 28, 2023 02:32
@stilist
Copy link
Author

stilist commented Jul 28, 2023

I have signed the CLA!

@Morriar
Copy link
Contributor

Morriar commented Jul 28, 2023

This looks like a duplicate of #716 which may need more work on the server part.

@stilist
Copy link
Author

stilist commented Jul 28, 2023

Ah, you're right. I'll close this.

@stilist stilist closed this Jul 28, 2023
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