Skip to content

various fixes for RubyParser#4166

Open
zenspider wants to merge 4 commits into
mainfrom
zenspider__rp_fixes
Open

various fixes for RubyParser#4166
zenspider wants to merge 4 commits into
mainfrom
zenspider__rp_fixes

Conversation

@zenspider

Copy link
Copy Markdown
Member

See individual commits for details.

One thing this doesn't have is the ACTUAL fixes I want for parser changes, because that would cause failures in the tests against RubyParser.

Is there a plan for how to deal with drift if I'm hoping to EOL RubyParser? Add them to known_failures?

Also, it looks like an import hasn't been for some time, and maybe the last one was done against git and not against a release... I'm hesitant to add/change those files as I'm not sure I'm doing the process correctly:

$ git clean -qf -- test/prism/fixtures ; rake seattlerb:import

@zenspider zenspider force-pushed the zenspider__rp_fixes branch 3 times, most recently from 5351eeb to e077752 Compare July 12, 2026 23:41
@zenspider zenspider force-pushed the zenspider__rp_fixes branch from e077752 to d98f0ab Compare July 12, 2026 23:44
@zenspider

Copy link
Copy Markdown
Member Author

Added an optional commit to update the tests to current. I can pull it to a different PR to keep this discussion scoped.

@Earlopain

Copy link
Copy Markdown
Collaborator

Generally adding failures to excludes in ruby_parser_test.rb somewhere is totally fine. It would be nice not to if possible but there are so many edge-cases, and often the result can also be different without actually affecting any consumer. I guess it depends on what drift you mean, that's not clear to me yet.

If I run the import, I also get pct_q_backslash_nl.txt, I think you might have forgotten to commit that one?

@zenspider

Copy link
Copy Markdown
Member Author

@Earlopain prolly gets swallowed as I'm on APFS and it is case insensitive by default... but since that commit broke EVERYTHING I should prolly punt it for now

@zenspider zenspider force-pushed the zenspider__rp_fixes branch from 47c3578 to d98f0ab Compare July 13, 2026 10:17
@kddnewton

Copy link
Copy Markdown
Collaborator

I'm seeing a couple of changes in here:

  1. Explicitly requiring timeout — ✅ makes sense
  2. Allowing passing scopes into the translator — ✅ makes sense
  3. Fixing the rake task regexp — ✅ makes sense
  4. Allowing subclassing the compiler — ?? this one I'm curious about. Not a big deal I suppose, but generally I try to avoid inheritance as much as possible so I'm curious about the use case

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