-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Display code block as plan text if Rouge raises an error #21
Conversation
Errors are found on analyzing the commit 3efe17e. 1 error:
We recommend to address them as possible, for example, update outdated dependencies, fix the tool's configuration, configure If you are struggling with these errors or warnings, feel free to ask us via chat. 💬 |
DisplayCopNames: true | ||
Exclude: | ||
- bin/**/* | ||
|
||
Metrics: | ||
Enabled: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like Metrics cops because they often introduce unnecessary refactoring, such as method splitting. So I disabled the department.
class RougeError < StandardError | ||
end | ||
|
||
def initialize(on_rouge_error: ->(ex) { warn ex }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rouge's error will be displayed to stderr by default.
User can configure it, for example, use Rails.logger
instead, or send the error to Sentry.
@@ -20,7 +20,7 @@ Gem::Specification.new do |spec| | |||
spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } | |||
spec.require_paths = ["lib"] | |||
|
|||
spec.required_ruby_version = ">= 2.3" | |||
spec.required_ruby_version = ">= 2.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RuboCop has dropped to support Ruby 2.3. So mate also dropped it.
Ruby 2.3 is too old, so I think we can drop to support it with only a few impacts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This pull request will suppress Rouge's error and display code block as plain text in SyntaxHighlight filter.
Problem
Sometimes Rouge raises an error due to Rouge's bug while highlighting. e.g.) rouge-ruby/rouge#1406
The error has a large impact on our service, Kibela.
Because the error conceals the whole of the note even if everything else works well.
Solution
Rescue Rouge's error, and use PlainText lexer instead.
By this change, the filter will use PlainText lexer instead of the appropriate language lexer if it raises an error.
PlainText lexer highlights nothing, but we can expect it raises an error very rarely.
So the user can see the note content even if Rouge raises an error.