Skip to content

fix: report an unknown theme by name instead of crashing - #2332

Open
lenamonj wants to merge 1 commit into
rouge-ruby:mainfrom
lenamonj:fix-unknown-theme-error
Open

lenamonj wants to merge 1 commit into
rouge-ruby:mainfrom
lenamonj:fix-unknown-theme-error

Conversation

@lenamonj

Copy link
Copy Markdown

rougify highlight -t <unknown-theme> dies with an unhandled NoMethodError instead of the error message the code already contains:

$ rougify highlight -t no-such-theme file.rb
lib/rouge/cli.rb:325:in 'initialize': undefined method 'new' for nil (NoMethodError)

The line is:

theme = Theme.find(opts[:theme]).new or error! "unknown theme #{opts[:theme]}"

.new binds tighter than or, so when Theme.find returns nil the crash happens before or is ever evaluated - the unknown theme message is unreachable. Splitting the lookup from the instantiation makes the intended message fire:

$ rougify highlight -t no-such-theme file.rb
unknown theme no-such-theme

Two lines changed, plus a spec in the existing Rouge::CLI::Highlight block asserting the Rouge::CLI::Error message. With the fix reverted that spec errors with the original NoMethodError; with it, rake check:specs is green at 1325 runs / 5804 assertions.

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.

1 participant