search

elm-community / SublimeElmLanguageSupport

Elm language syntax highlighting and tool integration for ST2/3.
https://packagecontrol.io/packages/Elm%20Language%20Support
MIT License
33 stars 12 forks source link

Syntax highlighting on GitHub broken #47

Open zwilias opened 6 years ago

zwilias commented 6 years ago

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 581b9e6f5bfdf55e506ac5a2b18422296c375ca2 to 6f2e53603a62663463f95079b308a1c9adbabeec 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 commented 6 years ago

🤔 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 commented 6 years ago

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 commented 6 years ago

@zwilias I think you should open an issue like this: https://github.com/github/linguist/issues/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 commented 6 years ago

I'm gonna put a temp fix in place.

eeue56 commented 6 years ago

@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?

zwilias commented 6 years ago

https://github.com/github/linguist/issues/4006 created. Thanks @eeue56!

kofigumbs commented 6 years ago

👋 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 commented 6 years ago

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 commented 6 years ago

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

kofigumbs commented 6 years ago

Kevin, that sounds solid to me. I didn't realize that's you were maintaining ST2 support here. Unless we hear otherwise from others I can look into updating linguist in the next week or two.

EDIT: I haven't gotten around to this, though I still think it's the proper approach. Hopefully someone else can find time to implement it.