Implement ARGF.class basics - #66
Open
edgibbs wants to merge 9 commits into
Open
Conversation
So it turned out ruby/spec requires identity, not just value: `ARGF.argv.equal?(ARGV)`
StdinIO just implements the write side, and raises IOError for puts/write as CRuby does matching CRuby 3.3.6. Didn't implement reading from stdin at all yet.
Opens the current file lazily, shifting its name off argv, and returns STDIN when no files were given. Not implemented yet is advancing to the next file at EOF.
Adds in the EOF advancing we deffered in teh ARGF#file commit earlier. So `read_byte()` moves to the next file when the current file is exhausted so the bytes run across all the files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TITLE: Implement ARGF.class basics
👋
Taking a run at #5. Same approach as last time, I used Claude Code with Opus 5 as a learning partner to find my way around the codebase and its patterns, but every line here I typed myself in Neovim, along with all the mistakes that entailed. This one took a lot longer than the last two, but figured it might be a challenge as an intermediate ticket. Apologies for the size, but it was a lot adding one thing and then uncovering a new small issue.
Both failures from the issue are gone:
The
String#each_bytefailure listed in the issue was already fixed by #56, so this only covers theARGFhalf.🔧 Implementation Notes
A few things worth calling out:
ARGF.argvhas to beARGVitself, not a copy.argv_spec.rb:10usesshould equal, and checking against MRI 3.3.6 the identity holds for the whole read cycle, even after the array has been emptied. Since it has to stay the same object,Argfholds theARGVRValueand shifts names off it in place. That's what changedArgf.new's signature and the smallexe/ruby-exe.tsdiff.Four of the commits are prerequisites the issue doesn't mention, which is why this touches six files:
IO#closed?andSTDIN— mspec'sargfhelper needs both in itsensure, so no ARGF spec could get past itIO#each_byte— what ARGF delegates toEnumerator#size— required byshared/each_byte.rb:49-55StdinIOimplements the write side only; nothing here reads from STDIN and a real readable one looked like a much bigger job.🧪 Testing
Beyond the repro above, across the whole
ruby/spec/core/argf/directory expectations passing went from 2 to 15 and errors dropped from 154 to 145. Failures went 1 to 2, but both of those are specs that used to error out in the helper and now run far enough to fail on other missing methods.📤 Upstream
While checking
Enumerator#sizeagainst MRI I noticedeach_byte_spec.rbonly tests non-empty strings, so an implementation returning nil for"".each_byte.sizepasses — which is exactly the bug I had, since 0 is falsy in JavaScript. MRI can't fail that case.each_char,each_codepoint, andeach_linehave no size specs at all. Planning a small ruby/spec PR for it.