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

Use Linguist lexers #153

Merged
merged 3 commits into from
Oct 5, 2014
Merged

Use Linguist lexers #153

merged 3 commits into from
Oct 5, 2014

Conversation

pchaigno
Copy link
Contributor

@pchaigno pchaigno commented Oct 2, 2014

This pull request uses Linguist instead of Pygments to determine which the correct lexer.
This would be more accurate since Linguist sometimes defines languages that Pygments doesn't know (and uses the lexer of another language, see M).

If this pull request is merged and the Linguist gem updated, it would solve the issue at github-linguist/linguist#1468.

/cc @bkeepers

@jch
Copy link
Contributor

jch commented Oct 2, 2014

@pchaigno thanks for the pull!

@mislav had opened a PR (#28 (comment)) in the past that tried to use linguist instead of pygments for detecting languages, but linguist does not recognize all the languages that pygments does. I'm not sure if that's still the case, but it would be good to have a test around this to prevent regressions.

@pchaigno
Copy link
Contributor Author

pchaigno commented Oct 2, 2014

Zut! Completely forgot to search in the closed PRs first...

Pygments still recognize some languages that Linguist doesn't.
I'll add a commit for backward compatibility.

@bkeepers
Copy link
Contributor

bkeepers commented Oct 3, 2014

👍 as long as it falls back to pygments

@pchaigno
Copy link
Contributor Author

pchaigno commented Oct 3, 2014

@jch I just added a commit to fall back to Pygments. Is that better?

lexer = Pygments::Lexer[lang]
end
lexer
end
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

def get_lexer(lang)
  (Linguist::Language[lang] && Linguist::Language[lang].lexer) || Pygments::Lexer[lang]
end

😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a more rubyish name?

def lexer_for(lang)
  (Linguist::Language[lang] && Linguist::Language[lang].lexer) || Pygments::Lexer[lang]
end

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I was going to suggest something similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback.
I changed that in my last commit.

@simeonwillbanks
Copy link
Contributor

👍

@jch jch added this to the 2.0 milestone Oct 5, 2014
jch added a commit that referenced this pull request Oct 5, 2014
@jch jch merged commit 68b876f into gjtorikian:master Oct 5, 2014
@pchaigno pchaigno deleted the linguist-lexers branch October 5, 2014 23:31
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