Skip to content

REFACTORING: supporting flag about receiving an offset or not.#722

Merged
headius merged 1 commit intojruby:masterfrom
josedonizetti:supporting_offset
May 10, 2013
Merged

REFACTORING: supporting flag about receiving an offset or not.#722
headius merged 1 commit intojruby:masterfrom
josedonizetti:supporting_offset

Conversation

@josedonizetti
Copy link
Member

This is important so we can fix all the specs related to write/binwrite. I've tried to make the change in a lot of ways but all of them look very ugly. So I decided to ask feedback from you that know more the code base. @Hedius @enebo

PS: write/binwrite specs need this because ruby doens't truncate the file an offset is given

headius added a commit that referenced this pull request May 10, 2013
REFACTORING: supporting flag about receiving an offset or not.
@headius headius merged commit d9a1e1d into jruby:master May 10, 2013
@headius
Copy link
Member

headius commented May 10, 2013

I'll fix the misspelling. Thanks!

@josedonizetti josedonizetti deleted the supporting_offset branch May 10, 2013 19:38
@atambo
Copy link
Member

atambo commented May 11, 2013

So I think what you are trying to do is satisfy the following sentence from the docs of IO.binwrite (http://ruby-doc.org/core-1.9.3/IO.html#method-c-binwrite)

If offset is not given, the file is truncated. Otherwise, it is not truncated.

You don't need to inject a boolean as a flag into all of these methods. The way this should be done is just add:

file.openFile.setMode(file.openFile.getMode() | ModeFlags.TRUNC);

inside of the IO.binwrite method in the right spot and the ChannelDescriptor will just handle the truncation for you.

I think we should just revert this commit and retry using the approach above.

@atambo
Copy link
Member

atambo commented May 11, 2013

So what I said above totally wasn't necessary it was as simple as changing the mode from "wb" to "r+w". See #726.

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.

3 participants