New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SASS Linter does not check syntax if it's installed using rbenv's gem #1819

Closed
Defman21 opened this Issue Jul 19, 2016 · 11 comments

Comments

Projects
None yet
3 participants
@Defman21
Contributor

Defman21 commented Jul 19, 2016

Short Summary

sass executable which has been installed through rbenv's gem is a bash script and it fails to run.
There's .rbenv/versions/2.3.0/lib/ruby/gems/2.3.0/gems/sass-3.4.22/bin/sass which is a Ruby script but it's not in PATH variable. I think Komodo should not try to run the executable using ruby, because the script contains #! declaration. Any shell will handle it properly.

https://github.com/Komodo/KomodoEdit/blob/master/src/lint/koCSSExLinter.py#L118

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Jul 19, 2016

The ruby is needed on Windows.

I installed Sass with my rbenv's gem ($ which gem yields "/home/mitchell/.rvm/rubies/ruby-2.3.0/bin/gem") and I'm successfully getting sass linter warnings and errors. ($ which sass yields "/home/mitchell/.rvm/gems/ruby-2.3.0/bin/sass", not the strange path you gave).

rbenv should automatically configure your PATH. For example, here's mine:

$ echo $PATH
/home/mitchell/.rvm/gems/ruby-2.3.0/bin:/home/mitchell/.rvm/gems/ruby-2.3.0@global/bin:/home/mitchell/.rvm/rubies/ruby-2.3.0/bin:/home/mitchell/.rvm/bin:...

I'm pretty sure there's a configuration issue on your part :(

@Defman21

This comment has been minimized.

Contributor

Defman21 commented Jul 19, 2016

You don't get it. I'm using rbenv which installed sass in a different folder which is not in PATH, but added a bash script sass in PATH.

$ which gem
/home/defman/.rbenv/shims/gem
$ which sass
/home/defman/.rbenv/shims/sass

These are sh scripts. ruby is required as an interpreter, but it could be omitted when you're calling sass. Would you mind do cat /home/mitchell/.rvm/gems/ruby-2.3.0/bin/sass?

Note: rbenv is a very popular replacement for rvm.

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Jul 19, 2016

$ cat `which sass`
#!/usr/bin/env ruby_executable_hooks
#
# This file was generated by RubyGems.
#
# The application 'sass' 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 'sass', version
load Gem.bin_path('sass', 'sass', version)

Wow, I didn't know there were two Ruby version managers. I thought rbenv was what I had. Oops, sorry for my ignorance :(

The fact that you added your own bash script that invokes your Sass is not something Komodo expects. Komodo expects the actual sass script/executable/whatever.

@Defman21

This comment has been minimized.

Contributor

Defman21 commented Jul 19, 2016

It's not my own but rbenv's :P
It could be fixed by reading the first line of the sass executable and if it starts with #!/usr/bin/env bash - then don't put ruby before it.

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Jul 19, 2016

I see. So:

  1. rbenv uses a shell script to invoke the Sass linter
  2. rvm uses a Ruby script to invoke the Sass linter.
  3. The Sass linter by itself is a Ruby script.

We will need to correctly identify an rbenv installation of Sass. Thanks for the clarification.

@mitchell-as mitchell-as added this to the Backlog milestone Jul 19, 2016

@mitchell-as mitchell-as self-assigned this Jul 19, 2016

@Defman21

This comment has been minimized.

Contributor

Defman21 commented Jul 19, 2016

You could simply check for which rbenv or which rvm. Not sure there are people who have both of them

Defman21 added a commit to Defman21/KomodoEdit that referenced this issue Jul 19, 2016

Linter: SCSS: detect rbenv and use it's sh file, otherwise use /path/…
…to/ruby /path/to/sass command; fixed Komodo#1819

Signed-off-by: Defman21 <i@defman.me>
@Naatan

This comment has been minimized.

Member

Naatan commented Sep 26, 2016

I use rvm :p I don't particularly min this fix, but I think it's a big rabbit hole if we choose to support all of these possible systems. And if we do intend to support them explicitly then we should have a better system for it (eg. some sort of API that returns executables, the API would take care of resolving them according to these systems).

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Sep 26, 2016

This is not a question of rvm vs rbenv. If you were to go through the previous discussion in this ticket, you'd see that rbenv uses shell scripts to invoke Ruby programs instead of using the Ruby interpreter. Since rvm uses Ruby to invoke Ruby programs, there's no need to provide explicit support for it -- our linter already invokes Ruby.

@Defman21

This comment has been minimized.

Contributor

Defman21 commented Sep 26, 2016

What Mitchell said. I don't know any other Ruby managers (? whatever) that are using sh to invoke Ruby except for rbenv. However, rbenv is quite popular in Ruby, so.. :)

Btw, may I know when the new nightly will come out with these fixes applied?

@Naatan

This comment has been minimized.

Member

Naatan commented Sep 26, 2016

Gotcha, thanks for explaining :)

@Defman21 no nightly planned atm, sorry. Best chance if you want something fast would be to backport it yourself.

@Defman21

This comment has been minimized.

Contributor

Defman21 commented Sep 26, 2016

Ohh ok. I'll do backporting then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment