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.

Sign up for GitHub

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

Jump to bottom

Allow read-only methods to work against a frozen StringIO #122

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Allow read-only methods to work against a frozen StringIO #122

wants to merge 4 commits into from

Conversation

headius
Copy link
Contributor

@headius headius commented Feb 20, 2025

Add tests and fix behavior to allow several read-only StringIO methods to function when the StringIO is frozen.

  • string (returns underlying String but does not mutate anything)
  • lineno
  • pos
  • closed?/closed_read?/closed_write?
  • eof/eof?
  • sync
  • pid (a dummy method but it writes nothing)
  • fileno (dummy)
  • pread (by definition does not modify state)
  • isatty/tty?
  • size/length
  • external_encoding
  • internal_encoding
  • initialize_copy (allow original to be frozen)

See #120 for details.

@headius
Don't require the string be modifiable to seek
2bbb856 
seek only modifies the StringIO pos, and should work when the
underlying String is frozen.

Fixes ruby#119
@headius
Add test for seek with frozen String
b946d46 
See ruby#119
@headius
Add tests for read-only methods on frozen StringIO
7bb821a 
See ruby#120
@headius
Copy link
Contributor Author

headius commented Feb 20, 2025

This is based on #121 and those commits will drop off when that PR is merged.

I would like input from @nobu @kou @hsbt @byroot since you all have related changes here. 🙇

@headius
Allow read-only methods to work with frozen StringIO
38303a4 
This fixes the JRuby extension to allow read-only methods to work
against a frozen StringIO, as described in ruby#119.
@headius
Copy link
Contributor Author

headius commented Feb 20, 2025

I can make the same change in the C extension (mostly switching a few StringIO() macro calls to check_strio() function calls, but @nobu @kou please advise me.

@headius headius mentioned this pull request Feb 20, 2025
Merged
@byroot
Copy link
Member

byroot commented Feb 21, 2025

I would like input

It would make sense to me that StringIO.new(str.freeze) would try to behave like a read only file. But I'm not StringIO maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@headius @byroot