-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
[Truffle] add RubyFiletypeDetector #3301
Conversation
|
||
public final class RubyFiletypeDetector extends FileTypeDetector { | ||
|
||
private final static String[] KNOWN_RUBY_FILES = new String[]{ "Gemfile", "Rakefile" }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we use Mavenfile for some projects (jruby-jars, jruby-openssl, etc) this "might" be another option. but my view of the world is biased :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Ping @jtulach |
What is the purpose? |
So GraalVM can be given a file with no more information and just run it. |
if (firstLine.isPresent() && firstLine.get().equals("#!/usr/bin/env ruby")) { | ||
return RUBY_MIME; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, but maybe too strict. What about two spaces in between "env" and "ruby". What if somebody uses just "#!/usr/bin/ruby"...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@pitr-ch No Java 8 method ;) |
final Optional<String> maybeFirstLine = Files.lines(path).findFirst(); | ||
if (maybeFirstLine.isPresent()) { | ||
final String firstLine = maybeFirstLine.get(); | ||
if (firstLine.matches("^#! ?/usr/bin/(env +ruby|ruby).*")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably best to compile the pattern only once.
4f40dde
to
6523b49
Compare
@@ -0,0 +1,50 @@ | |||
package org.jruby.truffle; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve forgot to ask, RubyFiletypeDetector
is currently in org.jruby.truffle
package, not the best place. What about org.jruby.truffle.runtime.util
or other new org.jruby.truffle.util
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FiletypeDetector isn't Truffle-only, it might be useful for whole JRuby - so consider option of putting it to generic JRuby runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes org.jruby might be right. And the metadata would then need to go in the core jar obviously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
087e726
to
b8a157e
Compare
No description provided.