-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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 to detect the language #537
Use Linguist to detect the language #537
Conversation
Gemfile
Outdated
@@ -11,3 +11,4 @@ gem "creole", "~>0.3.6" | |||
gem "wikicloth", "=0.8.1", :platforms => :ruby | |||
gem "asciidoctor", "= 1.5.2" | |||
gem "rake" | |||
gem "github-linguist" |
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.
For the purposes of getting this branch passing, you could use :github => "pchaigno/linguist", :branch => "blob_in_memory"
here.
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.
Done but the Travis build still fails because the samples.json
file of my Linguist fork isn't generated :/
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.
@bkeepers Anything I can do to fix that?
This is a great start! I'm curious, what if linguist was responsible for deciding which markup format to use? Something like def renderer(filename, content = nil)
blob = Linguist::Blob.new(filename, content)
if language = Linguist::Language.detect(blob)
markups.find { |impl| impl.name == language.markup_format }
end
end Enabling markup rendering for a language would just involve adding a Would that work? |
So for Textile files, would the markup_format be "Textile" or "Redcloth"? If the former, I'm not sure how that's different from the language name. If the latter, that seems mysteriously tied to the implementation of github/markup; i.e., someone adding or modifying a markup_format entry would need to know what renderers github/markup supports. |
402a7e8
to
fa488ce
Compare
I would think "Textile".
For the default case it wouldn't be, but it would allow other languages to opt-in to being rendered. |
33ca522
to
280b238
Compare
280b238
to
fa488ce
Compare
fa488ce
to
7828272
Compare
769fa63
to
6257172
Compare
b5db087
to
b1c90f5
Compare
64187a4
to
343d993
Compare
👋 Is this PR still something you're interested in working on? It looks like tests are passing; I think you merge master (and resolve conflicts) the one failing test in 343d993 is marked as allowed-to-fail in master! |
343d993
to
179819e
Compare
Definitely! I've updated the branch :-) |
I love it! ❤️ Thank you so much! |
This is now in use on the live site. Thanks a bunch! ✨ |
|
The Gemfile is different to the gemspec, I'm away from the computer at the moment but it either needs to be in both or the Gemfile can pull dependencies from the gemspec: http://yehudakatz.com/2010/12/16/clarifying-the-roles-of-the-gemspec-and-gemfile/ |
@andrew Thanks for that. I've added it to the gemspec accordingly. |
Linguist requires
|
@PikachuEXE I'm not quite sure what you mean by "Just cannot be installed" re: |
This PR breaks support for JRuby. This may be what @PikachuEXE was referring to, as both Of course, these errors on JRuby have gone unnoticed because it seems that So this ends up breaking gollum at least, and possibly more projects (such as @PikachuEXE 's). I must say this is rather disappointing and and unexpected behavior, especially since this was apparently not even considered a breaking change and was released as part of a patch-level update. I would appreciate it if these changes could be reverted in the |
@dometto This PR looks more like a new feature than a bug fix Forcing people to install those dependencies looks a bit "breaking" to me |
Understood. I think I'm 👍 on reverting the change in the final |
@kivikakk Off topic: |
@PikachuEXE hahaha :) Kobayashi and I have a lot in common, it turns out. |
This reverts commit bbee330.
Created #1033 to update change log |
Many thanks for the quick response and release! |
Terrific, thanks @kivikakk for resolving this so quickly! |
As discussed in github-linguist/linguist#2497, this pull requests makes use of
Linguist::Blob
to detect the language of a markup document.Linguist::Blob
is not yet part of Linguist and this pull request still has failing tests so it's work in progress./cc @bkeepers