Skip to content

Implement Dir.glob base option#4936

Merged
enebo merged 1 commit intojruby:ruby-2.5from
ChrisBr:ruby-2.5
Jan 5, 2018
Merged

Implement Dir.glob base option#4936
enebo merged 1 commit intojruby:ruby-2.5from
ChrisBr:ruby-2.5

Conversation

@ChrisBr
Copy link
Contributor

@ChrisBr ChrisBr commented Jan 4, 2018

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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wondering what the difference between runtime.getCurrentDirectory() and getCWD(runtime) or is it possible to simplify the code and only use either or?

Copy link
Member

Choose a reason for hiding this comment

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

@ChrisBr getCWD will force canonicalization whereas runtime.getCurrentDirectory is whatever it was set to (which might have already been canonicalized). Even if we happen to always canon. the stored value getCWD always tries to do it again (unless it is an URI).

Dir.glob is a dirty beast and I do not remember what contract it has for the incoming directory but that would be the proper guidance on which to use.

@enebo enebo added this to the JRuby 9.3.0.0 milestone Jan 5, 2018
@enebo enebo merged commit 4e6b330 into jruby:ruby-2.5 Jan 5, 2018
ChrisBr added a commit to ChrisBr/jruby that referenced this pull request Jan 6, 2018
now it throws an ArgumentError like it does in MRI.
Introduced in jruby#4936.
ChrisBr added a commit to ChrisBr/jruby that referenced this pull request Jan 9, 2018
now it throws an ArgumentError like it does in MRI.
Introduced in jruby#4936.
@enebo enebo modified the milestones: JRuby 9.3.0.0, JRuby 9.2.0.0 Apr 23, 2018
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.

2 participants