Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 632720ebe1ff
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: ae31c8201ad2
Choose a head ref
  • 2 commits
  • 2 files changed
  • 1 contributor

Commits on Mar 30, 2018

  1. Fix all ruby/spec failures for Dir.glob and Dir[] sans the one which … 

    …has an

obvious typo in it.  This work really makes me wish we had a nice piece of
infrastructure like rb_scan_args.  Argument processing with kwargs is pretty
awful.
    @enebo
    enebo committed Mar 30, 2018
    Copy the full SHA
    7f8df5c View commit details

  2. This has to have been fixed on ruby/spec?

    @enebo
    enebo committed Mar 30, 2018
    Copy the full SHA
    ae31c82 View commit details
Showing with 61 additions and 32 deletions.
  1. +60 −31 core/src/main/java/org/jruby/RubyDir.java
  2. +1 −1 spec/ruby/core/dir/glob_spec.rb
91 changes: 60 additions & 31 deletions core/src/main/java/org/jruby/RubyDir.java
View file
Original file line number Diff line number Diff line change
@@ -186,14 +186,61 @@ private static String getCWD(Ruby runtime) {
}
}

// returns null (no kwargs present), "" kwargs but no base key, "something" kwargs with base key (which might be "").
private static String globOptions(ThreadContext context, IRubyObject[] args, int[] flags) {
Ruby runtime = context.runtime;

if (args.length > 1) {
IRubyObject tmp = TypeConverter.checkHashType(runtime, args[args.length - 1]);
if (tmp.isNil()) {
if (flags != null) {
flags[0] = RubyNumeric.num2int(args[1]);
}
} else {
String[] keys = flags != null ? new String[] {"base", "flags"} : new String[] {"base"};
IRubyObject[] rets = ArgsUtil.extractKeywordArgs(context, (RubyHash) tmp, keys);
String base = rets[0].isNil() ? "" : RubyFile.get_path(context, rets[0]).asJavaString();

// Deep down in glob things are unhappy if base is not absolute.
if (!base.isEmpty()) {
JRubyFile file = new JRubyFile(base);

// FIXME: This is a bit of a hot mess. Deep down in the ancient caves of glob land we do not
// like see a base which is not an absolute path. JRubyFile by itself uses what I am guessing
// is the JVM CWD and not the runtimes CWD so if path is not absolute I remake the thing again...
if (!file.isAbsolute()) {
base = new JRubyFile(runtime.getCurrentDirectory(), base).getAbsolutePath();
}
}
if (flags != null) flags[0] = rets[1].isNil() ? 0 : RubyNumeric.num2int(rets[1]);
return base;
}
}

return null;
}

@JRubyMethod(name = "[]", rest = true, meta = true)
public static IRubyObject aref(ThreadContext context, IRubyObject recv, IRubyObject[] args) {
Ruby runtime = context.runtime;
String base = globOptions(context, args, null);
List<ByteList> dirs;

if (args.length == 1) {
dirs = Dir.push_glob(runtime, getCWD(runtime), globArgumentAsByteList(context, args[0]), 0);
String dir = base == null || base.isEmpty() ? runtime.getCurrentDirectory() : base;
dirs = Dir.push_glob(runtime, dir, globArgumentAsByteList(context, args[0]), 0);
} else {
dirs = dirGlobs(context, getCWD(runtime), args, 0);
IRubyObject[] arefArgs;
if (base != null) { // kwargs are present
arefArgs = new IRubyObject[args.length - 1];
System.arraycopy(args, 0, arefArgs, 0, args.length - 1);
} else {
arefArgs = args;
base = "";
}

String dir = base.isEmpty() ? runtime.getCurrentDirectory() : base;
dirs = dirGlobs(context, dir, arefArgs, 0);
}

return asRubyStringList(runtime, dirs);
@@ -212,39 +259,21 @@ private static ByteList globArgumentAsByteList(ThreadContext context, IRubyObjec
@JRubyMethod(required = 1, optional = 2, meta = true)
public static IRubyObject glob(ThreadContext context, IRubyObject recv, IRubyObject[] args, Block block) {
Ruby runtime = context.runtime;
int flags = 0;
String dir, base = "";

if(args.length == 3) {
flags = RubyNumeric.num2int(args[1]);
IRubyObject tmp = TypeConverter.checkHashType(runtime, args[2]);
if (tmp.isNil()){
throw runtime.newArgumentError("wrong number of arguments (" + args.length + " for 1..2)");
} else {
IRubyObject[] rets = ArgsUtil.extractKeywordArgs(context, (RubyHash) args[2], "base");
base = rets[0].convertToString().toString();
}
} else if(args.length == 2) {
IRubyObject tmp = TypeConverter.checkHashType(runtime, args[1]);
if(tmp.isNil()){
flags = RubyNumeric.num2int(args[1]);
} else {
IRubyObject[] rets = ArgsUtil.extractKeywordArgs(context, (RubyHash) args[1], "base");
base = rets[0].convertToString().toString();
}
}

int[] flags = new int[] { 0 };
String base = globOptions(context, args, flags);
List<ByteList> dirs;
IRubyObject tmp = args[0].checkArrayType();

if (!base.isEmpty() && !(new File(base).exists())){
if (!(base == null || base.isEmpty()) && !(new File(base).exists())){
dirs = new ArrayList<ByteList>();
} else if (tmp.isNil()) {
dir = base.isEmpty() ? runtime.getCurrentDirectory() : base;
dirs = Dir.push_glob(runtime, dir, globArgumentAsByteList(context, args[0]), flags);
} else {
dir = base.isEmpty() ? getCWD(runtime) : base;
dirs = dirGlobs(context, dir, ((RubyArray) tmp).toJavaArray(), flags);
IRubyObject tmp = args[0].checkArrayType();
String dir = base == null || base.isEmpty() ? runtime.getCurrentDirectory() : base;

if (tmp.isNil()) {
dirs = Dir.push_glob(runtime, dir, globArgumentAsByteList(context, args[0]), flags[0]);
} else {
dirs = dirGlobs(context, dir, ((RubyArray) tmp).toJavaArray(), flags[0]);
}
}

if (block.isGiven()) {
2 changes: 1 addition & 1 deletion spec/ruby/core/dir/glob_spec.rb
View file
Original file line number Diff line number Diff line change
@@ -169,7 +169,7 @@
ruby_version_is "2.5" do
it "accepts base: parameter" do
Dir.mkdir 'foo'
dir = File.join(@cwd, foo)
dir = File.join(@cwd, 'foo')
files = %w[a/foo.c c/bar.c].map {|n| File.join(dir, n)}
files.each {|n| File.write(n, "")}
assert_equal(files, Dir.glob("*/*.c", base: dir))