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

File.open ignores :universal_newline options on Windows #3762

Closed
headius opened this issue Mar 28, 2016 · 2 comments
Closed

File.open ignores :universal_newline options on Windows #3762

headius opened this issue Mar 28, 2016 · 2 comments

Comments

@headius
Copy link
Member

@headius headius commented Mar 28, 2016

Duplicated from #3736 for 9k specific work.

Environment

both jruby 1.7.24 and JRuby 9.0.5.0 have this issue.

Expected Behavior

A file which contains \r\n should still retain the \r\n and not convert to \n if universal_newline is false.

require 'csv'
require "tempfile"

@tempfile = Tempfile.new(%w"temp .csv")
@tempfile.close
@path = @tempfile.path

File.open(@path, "wb") do |file|
  file << "1\t2\t3\r\n"
  file << "4\t5\r\n"
end

f = File.open(@path, {:universal_newline=>false})
p f.gets

Actual Behavior

We seem to always return \n as delimiter.

@headius
Copy link
Member Author

@headius headius commented Mar 28, 2016

Ok, I think I see the issue in 9k.

Because we can't open a file in text mode on Windows (Java has no such concept) we currently force read/write conversion to use UNIVERSAL_NEWLINE_DECORATOR. Your example, explicitly tries to turn that mode off, but since we're on Windows doing text mode it gets turned back on again.

So we need a way to propagate that universal newlines were explicitly disabled and not turn them on for text mode in that case.

It may be the case that existing processing of textmode during ecflags handling is doing everything we need. Inside EncodingUtils.extractModeEncoding I see it setting universal newline whenever we're in textmode, but with an eye for incoming options overriding that. Investigating.

@headius
Copy link
Member Author

@headius headius commented Mar 28, 2016

Possible fix that appears to work for various combinations of modes: https://gist.github.com/headius/642d4895873f28ee806b

I believe @enebo is going to verify. I am taking a step back to attempt to get specs running on Windows in some usable form.

@headius headius closed this in b73e950 Mar 28, 2016
headius added a commit that referenced this issue Mar 28, 2016
@headius headius added the windows label Mar 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.