ianhattendorf / autocomplete-ruby

Provides intelligent code completion for Ruby in the Atom editor. Requires RSense.
MIT License
51 stars 9 forks source link

Attempt to be more agnostic when fetching gem directory #19

Closed chenasraf closed 7 years ago

chenasraf commented 7 years ago

I'm not really able to test it, but this is the general direction needed to use the correct path for rsense. Ideally either the first or second method should work, but the third is there for extra safety (though it will probably not work on Windows)

ianhattendorf commented 7 years ago

This looks great so far, thank you. I understand not being able to do extensive testing, are you using it yourself? If so, what OS/ruby environment (version/rbenv/etc.)? Ideally I'd like to ensure it's working on linux/macOS/windows if at all possible.

chenasraf commented 7 years ago

I'm on OSX Sierra using rbenv. Here's some version info:

➜  ~ sw_vers
ProductName:    Mac OS X
ProductVersion: 10.12
BuildVersion:   16A323
➜  ~ rbenv --version
rbenv 1.0.0
➜  ~ ruby --version
ruby 2.3.1p112 (2016-04-26 revision 54768) [x86_64-darwin15]
➜  ~ bundler --version
Bundler version 1.13.1
➜  ~ gem --version
2.5.1

I added a checklist to the post for what I think would be needed for this to be production ready

ianhattendorf commented 7 years ago

Great, thanks. What are the blockers for Windows? Do you just need someone to test it?

chenasraf commented 7 years ago

@ianhattendorf I updated the PR, should work properly now (there were problems with the previous implementation). Still untested for Windows, not a lot I can do about it right now, would love for someone else to contribute.

elliotcm commented 7 years ago

@chenasraf I'm not a regular ruby-on-windows user but I have a windows 10 machine available so I tried this out.

I installed Atom fresh and installed ruby 2.3.3 using rubyinstaller which seems to be the main way it gets done (except for people who run ubuntu within windows at which point it's just linux). I then did gem install rsense.

I got the following error when attempting to write some ruby: snap 2017-01-23 at 13 00 47

Note the /bin/bin. The rsense executable is located at C:/Ruby23-x64/bin/rsense.

chenasraf commented 7 years ago

@elliotcm Can you run gem environment in a command line and paste the output? Or if you're worried about sensitive info, just the path on the line of EXECUTABLE DIRECTORY should be enough, though, if you find another entry there that has the correct path - paste it too, that would be great. Thanks!

elliotcm commented 7 years ago

@chenasraf EXECUTABLE DIRECTORY is set to C:/Ruby23-x64/bin which seems comparable to the output I'm seeing on OSX.

If you want more fields I don't mind providing them, just I don't have a shared clipboard between the machines.

Edit: I got around to logging in on windows, see output below.

elliotcm commented 7 years ago
ruby 2.3.3p222 (2016-11-21 revision 56859) [x64-mingw32]
C:\Users\Elliot>gem environment
RubyGems Environment:
  - RUBYGEMS VERSION: 2.5.2
  - RUBY VERSION: 2.3.3 (2016-11-21 patchlevel 222) [x64-mingw32]
  - INSTALLATION DIRECTORY: C:/Ruby23-x64/lib/ruby/gems/2.3.0
  - USER INSTALLATION DIRECTORY: C:/Users/Elliot/.gem/ruby/2.3.0
  - RUBY EXECUTABLE: C:/Ruby23-x64/bin/ruby.exe
  - EXECUTABLE DIRECTORY: C:/Ruby23-x64/bin
  - SPEC CACHE DIRECTORY: C:/Users/Elliot/.gem/specs
  - SYSTEM CONFIGURATION DIRECTORY: C:/ProgramData
  - RUBYGEMS PLATFORMS:
    - ruby
    - x64-mingw32
  - GEM PATHS:
     - C:/Ruby23-x64/lib/ruby/gems/2.3.0
     - C:/Users/Elliot/.gem/ruby/2.3.0
  - GEM CONFIGURATION:
     - :update_sources => true
     - :verbose => true
     - :backtrace => false
     - :bulk_threshold => 1000
  - REMOTE SOURCES:
     - https://rubygems.org/
  - SHELL PATH:
     - C:\Ruby23-x64\bin
[... bunch of system32 paths...]
     - C:\Ruby23-x64\bin
     - C:\Users\Elliot\AppData\Local\atom\bin
chenasraf commented 7 years ago

@elliotcm Alright my bad, I added bin forcibly even though it wasn't necessary. Default path should work now, if you can confirm, I'll check the second box and this should be ready for merging into the main branch

elliotcm commented 7 years ago

@chenasraf @ianhattendorf This now finds the correct executable on windows, which gives me the same error as this guy: https://github.com/ianhattendorf/autocomplete-ruby/issues/17#issuecomment-265720456

ianhattendorf commented 7 years ago

Great job guys! @chenasraf, does this still work on linux/macOS after removing the additional bin?

chenasraf commented 7 years ago

@ianhattendorf yeah, it was an error on my part :) should be good to merge

ianhattendorf commented 7 years ago

Great, I'll merge it now and release shortly.

vzamanillo commented 7 years ago

Hi!

It doesn't work with rvm

screen

running gem environment from the console works as expected

2

and the EXECUTABLE DIRECTORY ~/.rvm/gems/ruby-2.3.1/bin/rsense (in my case) is not the correct path

3

it should be ~/.rvm/gems/ruby-2.3.1/wrappers/rsense

In any case, the possible execution errors from stderr should be checked to return the default path and leave the package working.

ianhattendorf commented 7 years ago

Could you post the output of gem environment? Thanks.

vzamanillo commented 7 years ago

Of course,

RubyGems Environment:
  - RUBYGEMS VERSION: 2.5.1
  - RUBY VERSION: 2.3.1 (2016-04-26 patchlevel 112) [x86_64-linux]
  - INSTALLATION DIRECTORY: /home/vzamanillo/.rvm/gems/ruby-2.3.1
  - USER INSTALLATION DIRECTORY: /home/vzamanillo/.gem/ruby/2.3.0
  - RUBY EXECUTABLE: /home/vzamanillo/.rvm/rubies/ruby-2.3.1/bin/ruby
  - EXECUTABLE DIRECTORY: /home/vzamanillo/.rvm/gems/ruby-2.3.1/bin
  - SPEC CACHE DIRECTORY: /home/vzamanillo/.gem/specs
  - SYSTEM CONFIGURATION DIRECTORY: /home/vzamanillo/.rvm/rubies/ruby-2.3.1/etc
  - RUBYGEMS PLATFORMS:
    - ruby
    - x86_64-linux
  - GEM PATHS:
     - /home/vzamanillo/.rvm/gems/ruby-2.3.1
     - /home/vzamanillo/.rvm/gems/ruby-2.3.1@global
  - GEM CONFIGURATION:
     - :update_sources => true
     - :verbose => true
     - :backtrace => false
     - :bulk_threshold => 1000
     - :sources => ["http://*******/repository/rubygems-group/"]
  - REMOTE SOURCES:
     - http://******/repository/rubygems-group/
  - SHELL PATH:
     - /home/vzamanillo/.rvm/gems/ruby-2.3.1/bin
     - /home/vzamanillo/.rvm/gems/ruby-2.3.1@global/bin
     - /home/vzamanillo/.rvm/rubies/ruby-2.3.1/bin
     - /home/vzamanillo/bin
     - /home/vzamanillo/.local/bin
     - /home/vzamanillo/bin
     - /home/vzamanillo/.local/bin
     - /usr/local/sbin
     - /usr/local/bin
     - /usr/sbin
     - /usr/bin
     - /sbin
     - /bin
     - /usr/games
     - /usr/local/games
     - /home/vzamanillo/.rvm/bin
     - /home/vzamanillo/bin
     - /home/vzamanillo/.rvm/bin

Thank you

ianhattendorf commented 7 years ago

Hmm, I don't see anything with /wrappers/ in it, looks like we would have to detect if RVM is being used and use {INSTALLATION DIRECTORY}/wrappers/rsense. @chenasraf, any ideas?

In this case, there is no default path that would work for you. The default path is just ~/.gem/ruby/2.3.0/bin/rsense, which is incorrect in your case. I think it would nice to have a more helpful error, pointing to the readme for correct setup.

You can set the rsensePath config option in the plugin settings to your rsense path and it should work.

elliotcm commented 7 years ago

To add some more confusion here, with rbenv:

screenshot 2017-01-26 20 55 59

I'm going to stick /Users/elliot/.rbenv/versions/2.3.1/bin/rsense in the rsensePath setting and leave it at that, I think.

chenasraf commented 7 years ago

@vzamanillo Does which rsense output anything useful? I find it odd that RVM doesn't place things at their expected directory, or at least symlink... Not sure what I can do about it without a programmable way to get the correct dir

vzamanillo commented 7 years ago

rsense is located at

~/.rvm/gems/ruby-2.3.1/bin/rsense

and this is what the script does

#!/usr/bin/env ruby_executable_hooks
#
# This file was generated by RubyGems.
#
# The application 'rsense' is installed as part of a gem, and
# this file is here to facilitate running it.
#

require 'rubygems'

version = ">= 0.a"

if ARGV.first
  str = ARGV.first
  str = str.dup.force_encoding("BINARY") if str.respond_to? :force_encoding
  if str =~ /\A_(.*)_\z/ and Gem::Version.correct?($1) then
    version = $1
    ARGV.shift
  end
end

gem 'rsense', version
load Gem.bin_path('rsense', 'rsense', version)

and this is what the wrapper (at ~/.rvm/gems/ruby-2.3.1/wrappers/rsense) does

#!/usr/bin/env bash

if
  [[ -s "/home/vzamanillo/.rvm/gems/ruby-2.3.1/environment" ]]
then
  source "/home/vzamanillo/.rvm/gems/ruby-2.3.1/environment"
  exec rsense "$@"
else
  echo "ERROR: Missing RVM environment file: '/home/vzamanillo/.rvm/gems/ruby-2.3.1/environment'" >&2
  exit 1
fi
chenasraf commented 7 years ago

@ianhattendorf do you think we can just use which rsense instead of parsing gem environment? The only possible issue I could think of is that rsense might not be globally loaded as a bin for all shell environments; then again, we could say the same about gem and it works well

ianhattendorf commented 7 years ago

Easiest way would probably be to add another step to the detection:

rsensePath default is "#{GEM_HOME}/rsense", we could test which rsense, store the result in a variable, and set the default to whichRsensePath ? "#{GEM_HOME}/rsense".

I do remember there were some issues with Atom not correctly loading the path when launched from the desktop icon, and having to be launched from the shell.