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

Syntax highlighting on GitHub broken #47

Open
zwilias opened this issue Jan 27, 2018 · 10 comments
Open

Syntax highlighting on GitHub broken #47

zwilias opened this issue Jan 27, 2018 · 10 comments

Comments

@zwilias
Copy link
Member

zwilias commented Jan 27, 2018

Interestingly, GitHub uses this very repo as the provider for syntax highlighting on GitHub. https://github.com/github/linguist/blob/v6.0.0/.gitmodules#L679-L681

In the latest update of linguist, they updated all the submodules. The Elm module went from 581b9e6 to 6f2e536 which is a fairly big change.

It would be pretty great if we could figure out a way forward - whether that means making this repo compatible with linguist or something else, I'll leave in the middle as I have no real idea how this bundle actually works.

@sentience
Copy link

sentience commented Jan 27, 2018

🤔 I hadn’t heard of Linguist until you mentioned it, and having skimmed its README it isn’t clear to me that linguist does do syntax highlighting for GitHub—it sounds like it is simply a tool for detecting the programming language present in a file. If you’re sure the reason it depends on this project, though, is for syntax highlighting, I’ll take your word for it!

The primary purpose for this project is to provide support for the Elm language to Sublime Text 3 or later (with previous versions supported by a legacy branch). Because Sublime Text 3 supports a superior language syntax file format than the old TextMate .tmLanguage files, we have migrated to this new format in this project. I suspect GitHub requires the old .tmLanguage file, which is no longer present in this project.

GitHub could probably adjust its submodule reference to point to the st2-0.19.0 tag, which would make the .tmLanguage file available again. It’s probably fine for GitHub to depend on this legacy branch for the time being. When Elm next introduces significant syntax changes, though, someone will need to go back and update that branch to keep Linguist compatible with new versions of Elm.

Does this sound doable, @zwilias?

@zwilias
Copy link
Member Author

zwilias commented Jan 27, 2018

Yeah, Linguist essentially does both - language recognition using a built in set of rules, and based on the recognised language, highlighting using a bunch of vendored grammars.

Cool, the switch to ST3's new syntax format would indeed explain this. I'm wondering about the best next course of action. Ultimately, having a supported TextMate bundle would be best (for as long as GitHub uses that for highlighting), though I wouldn't wish that extra maintenance cost on anyone.

Perhaps it would be best to create a separate repo for a "legacy" tmLanguage definition, so at least the maintenance doesn't become your responsibility by default.

CC/ @eeue56 do you have any thoughts on this?

@eeue56
Copy link

eeue56 commented Jan 27, 2018

@zwilias I think you should open an issue like this: github-linguist/linguist#4004 and discuss there. It might be a similar issue, in which case I would prefer us to maintain a single source of truth for syntax highlighting for Elm. If ST3 syntax highlighting isn't supported by linguist, we should indeed make a separate repo.

I don't see anything the linguist repo explicitly stating that they require .tmLanguage in the docs. So it might be that they support .sublime_syntax too.

@eeue56
Copy link

eeue56 commented Jan 27, 2018

I'm gonna put a temp fix in place.

@eeue56 eeue56 self-assigned this Jan 27, 2018
@eeue56
Copy link

eeue56 commented Jan 27, 2018

@zwilias Okay so I made Elm.tmLanguage during this interim while figuring out what to do. It's pointing again to that old commit: https://github.com/elm-community/Elm.tmLanguage. Can you ask them to do a fresh pull from there?

@eeue56 eeue56 removed their assignment Jan 27, 2018
@zwilias
Copy link
Member Author

zwilias commented Jan 27, 2018

github-linguist/linguist#4006 created. Thanks @eeue56!

@kofigumbs
Copy link

👋 I came across this issue, because I was trying to implement a small fix for GitHub syntax highlighting. I'm wondering if anyone in this conversation could clarify which repo we intend to be the source of truth for GitHub highlighting.

It seems that GitHub will not support the new Sublime syntax, so keeping Elm.tmLanguage around specifically for GitHub seems reasonable. In that case, it might be nice to strip out the Sublime 2 stuff, to communicate to future visitors that st2-0.19.0 is probably what they want. GitHub does support the Atom format, so telling Linguist to look there might make sense as well. Those are the only options I could think of, but I'm guessing there are more.

I'm willing to do the work, so just let me know what you all are thinking.

@sentience
Copy link

We’ll be maintaining st2- releases for the foreseeable future for Sublime Text 2 users, and I would say the most reasonable way to do that will be with a st2 branch. If I set that up, GitHub Syntax Highlighting could point to that branch. Sound good, @hkgumbs?

@sentience
Copy link

Alternatively, @eeue56 if you think it makes more sense to maintain Elm.tmLanguage as a separate project, can we find it a maintainer?

@kofigumbs
Copy link

kofigumbs commented Apr 8, 2018 via email

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

No branches or pull requests

4 participants