Skip to content

Conversation

@zenspider
Copy link
Member

I'd rather there be no require whatesoever as the SyntaxError class
there is basic af... but this guard should do the trick.

@zenspider
Copy link
Member Author

I have a failure in rust bindings. That's green on main. Is is flaky or what?

@zenspider zenspider force-pushed the zenspider__fix_ruby_parser_dependency branch 3 times, most recently from b91bac2 to 9389843 Compare December 13, 2025 01:04
@zenspider zenspider requested a review from kddnewton December 13, 2025 01:04
@zenspider zenspider changed the title Only require ruby_parser if RubyParser::SyntaxError is undefined Define RubyParser::SyntaxError so we don't have to require ruby_parser anymore Dec 13, 2025
rescue LoadError
warn(%q{Error: Unable to load ruby_parser. Add `gem "ruby_parser"` to your Gemfile.})
exit(1)
require "sexp"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's tweak the begin/rescue instead of removing it. I think I am ok to drop the hard requirement on ruby_parser since it only depends on that one exception but the error message for missing sexp_processor should still be present.

require "sexp"

class RubyParser # :nodoc:
SyntaxError = Class.new RuntimeError # :nodoc:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This avoids a redeclared constant warning if ruby_parser ends up being loaded first anyways.

Suggested change
SyntaxError = Class.new RuntimeError # :nodoc:
class SyntaxError < RuntimeError; end # :nodoc:

…ser.

Had to add a require of sexp since that came in indirectly via ruby_parser.
@kddnewton kddnewton force-pushed the zenspider__fix_ruby_parser_dependency branch from 9389843 to df677c3 Compare December 14, 2025 16:23
@kddnewton kddnewton merged commit 122c80a into main Dec 14, 2025
64 checks passed
@kddnewton kddnewton deleted the zenspider__fix_ruby_parser_dependency branch December 14, 2025 16:42
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.

4 participants