-
Notifications
You must be signed in to change notification settings - Fork 283
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
Support syntax highlighting for multiple languages #110
Conversation
@@ -122,7 +122,7 @@ def format_failures(failures_per_suite) | |||
def format_failure(f) | |||
" #{f[:test_case]}, #{red(f[:reason])}\n #{cyan(f[:file_path])}\n" + | |||
" ```\n" + | |||
Syntax.highlight(Snippet.from_filepath(f[:file_path])) + | |||
Syntax.highlight(Snippet.from_filepath(f[:file_path]), nil, File.basename(f[:file_path])) + |
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.
Line is too long. [97/80]
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.
@kattrali what if we reorder options
and filetype, then you wouldn't have to pass explicit nil
?
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.
@kattrali on the side note, i think Snippet
shouldn't be a String
. This was a quick hack I've done before, but if it was a better object, it'd remember the file_path
when you initialize it.
The usage:
snippet = Snippet.from_filepath(f[:file_path])
snippet.contents #=> String
snippet.file_path #=> String /Users/..../file.m:234
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.
Yeah, that seems much cleaner, then we don't need an extra parameter at all.
@kattrali thanks for working on this! I'd appreciate if you add tests for it, I can do the refactors if you want |
@supermarin sounds good, wanted to make sure it was the right direction first 👍 |
RUBY = 'ruby' | ||
RUBY_EXTENSIONS = ['.ruby','.rb'] | ||
|
||
def self.highlight(code, options="", filename="") |
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.
Surrounding space missing in default value assignment.
Prefer single-quoted strings when you don't need string interpolation or special symbols.
RUBY = 'ruby' | ||
RUBY_EXTENSIONS = ['.ruby', '.rb'] | ||
|
||
def self.highlight(snippet, options = "") |
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.
Prefer single-quoted strings when you don't need string interpolation or special symbols.
|
||
it 'highlights dylan code by filename' do | ||
snippet = Snippet.new( | ||
'define method write-end-array (serializer :: <json-serializer>)', |
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.
Trailing whitespace detected.
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.
WHERE
Cleanup contants definition
ext = File.extname(filename) | ||
@filetypes[ext] || 'objc' | ||
end | ||
|
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.
Extra empty line detected at body end.
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.
hush you
end | ||
end | ||
|
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.
1 trailing blank lines detected.
@kattrali shouldn't we enforce a trailing line on EOF? |
@supermarin yeah, I think so |
Previously, we were basically testing the behavior of Pygments. Now we're just testing if Pygments are being called with the right arguments.
Speed up tests a lot 🚀
end | ||
end | ||
end | ||
|
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.
1 trailing blank lines detected.
It's time. |
Support syntax highlighting for multiple languages
Add an optional
language
argument topygmentize
, doing a dumb guess as to a file's language. Default language remains objc.