Skip to content
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

Merged
merged 1 commit into from
Sep 7, 2015
Merged

[Truffle] add RubyFiletypeDetector #3301

merged 1 commit into from
Sep 7, 2015

Conversation

pitr-ch
Copy link
Member

@pitr-ch pitr-ch commented Sep 2, 2015

No description provided.

@pitr-ch pitr-ch added the truffle label Sep 2, 2015
@pitr-ch pitr-ch self-assigned this Sep 2, 2015
@pitr-ch pitr-ch added this to the truffle-dev milestone Sep 2, 2015

public final class RubyFiletypeDetector extends FileTypeDetector {

private final static String[] KNOWN_RUBY_FILES = new String[]{ "Gemfile", "Rakefile" };
Copy link
Member

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 :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@chrisseaton
Copy link
Contributor

Ping @jtulach

@eregon
Copy link
Member

eregon commented Sep 2, 2015

What is the purpose?

@chrisseaton
Copy link
Contributor

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;
}

Copy link
Contributor

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"...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@eregon
Copy link
Member

eregon commented Sep 2, 2015

@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).*")) {
Copy link
Member

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.

@pitr-ch pitr-ch force-pushed the master branch 4 times, most recently from 4f40dde to 6523b49 Compare September 4, 2015 09:08
@@ -0,0 +1,50 @@
package org.jruby.truffle;
Copy link
Member Author

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?

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@pitr-ch pitr-ch force-pushed the master branch 3 times, most recently from 087e726 to b8a157e Compare September 7, 2015 09:41
@pitr-ch pitr-ch merged commit 410350c into jruby:master Sep 7, 2015
@enebo enebo added this to the Non-Release milestone Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants