Skip to content
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

Add Xojo lexer. #1131

Merged
merged 6 commits into from
May 29, 2019
Merged

Add Xojo lexer. #1131

merged 6 commits into from
May 29, 2019

Conversation

jimmckay
Copy link
Contributor

Started with a fresh branch.
Simplified the lexer and removed unneeded parsing.

Started with a fresh branch.
Simplified the lexer and removed unneeded parsing.
@pyrmont pyrmont mentioned this pull request May 28, 2019
@pyrmont
Copy link
Contributor

pyrmont commented May 28, 2019

@jimmckay There's an error in the code that's causing Travis to fail. You should be able to run the same tests locally first before submitting. Running rake spec and rubocop from the root of the project should do the trick.

@pyrmont pyrmont added the author-action The PR has been reviewed but action by the author is needed label May 28, 2019
@pyrmont
Copy link
Contributor

pyrmont commented May 28, 2019

Thanks for responding so quickly, @jimmckay. I'm looking through the lexer but have an additional request in the meantime. Could you add a spec? A simple one like the one for Rust would be fine:

# -*- coding: utf-8 -*- #
# frozen_string_literal: true
describe Rouge::Lexers::Rust do
let(:subject) { Rouge::Lexers::Rust.new }
describe 'guessing' do
include Support::Guessing
it 'guesses by filename' do
assert_guess :filename => 'foo.rs'
end
it 'guesses by mimetype' do
assert_guess :mimetype => 'text/x-rust'
end
it 'guesses by source' do
assert_guess :source => '#!/usr/bin/env rustc --jit'
end
end
end

You can skip the guessing by source test as it's not relevant here.

@pyrmont pyrmont added the needs-review The PR needs to be reviewed label May 28, 2019
@jimmckay
Copy link
Contributor Author

Sure. I'll take a look.

@pyrmont
Copy link
Contributor

pyrmont commented May 28, 2019

Cheers :) Might not be able to get through a review tonight but will try to have it done by tomorrow!

@pyrmont pyrmont removed the author-action The PR has been reviewed but action by the author is needed label May 28, 2019
@jimmckay
Copy link
Contributor Author

Sounds great! Let me know about anything that could be improved. I'm very new to Ruby.

lib/rouge/lexers/xojo.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/xojo.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/xojo.rb Show resolved Hide resolved
lib/rouge/lexers/xojo.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/xojo.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/xojo.rb Outdated Show resolved Hide resolved
@pyrmont pyrmont added author-action The PR has been reviewed but action by the author is needed and removed needs-review The PR needs to be reviewed labels May 29, 2019
jimmckay added 2 commits May 28, 2019 18:13
Added an intentional error to test for 1..0 being read as a valid float
Removed state sections. This lexer is simple enough that states are really not needed at this point.
Added word boundary to the Float regex to avoid 1..0 being read as a valid float.
Adjusted double-quote regex to allow for (and enforce) escaped quotes (two double quotes with no space between) I may revisit this in the future.
&HFFEEDD style literals now marked as Literal.Number.Hex rather than Integer
@pyrmont pyrmont added needs-review The PR needs to be reviewed and removed author-action The PR has been reviewed but action by the author is needed labels May 29, 2019
@pyrmont pyrmont merged commit 3de0835 into rouge-ruby:master May 29, 2019
@pyrmont pyrmont removed the needs-review The PR needs to be reviewed label May 29, 2019
@pyrmont
Copy link
Contributor

pyrmont commented May 29, 2019

Thanks for putting this together, @jimmckay! Much appreciated :)

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.

2 participants