Pull out reasonml integration?

[chenglou]

Hello! I'm a maintainer of Reason. @gmmorris has created to the reasonml highlight recently, and in order to iterate on it faster, I've pulled it out into https://github.com/reasonml-editor/reason-highlightjs. I've been using it in place of the first-party one, and it's been solid.
Pulling the plugin out also makes it so that folks don't have to build highlightjs master to obtain it (which isn't always easily doable since it might be a transitive dependency). Last release was a year ago. Additionally, it seems that highlightjs would like to externalize the language plugins anyway; this would already be done.
So I'm wondering if we should just remove the reasonml plugin from this repo; if not, then I can sync that plugin back in if you'd like.

Thanks!

[marcoscaceres]

We should sync once and start emitting a warning. I’d love to hear more about the externalization approach. Really think it’s the way to go.

[chenglou]

Oh, I'm just mentioning it because I saw it in #1759. If that's not in progress then never mind.

[marcoscaceres]

Yeah, not in progress... need to look at the architecture for how to actually do it ☺️ I'm hoping to find folks in the community who can help.

[gmmorris]

While I completely support the approach, my concern would be that until it's a ubiquitous way of plugging in languages, it'll just create confusion.

It would only be a matter of time until someone else adds reason support to master without knowing the plug in exists.

This should be done as a whole refractor, extracting other key languages.

[marcoscaceres]

It would only be a matter of time until someone else adds reason support to master without knowing the plug in exists.

As the project maintainers would not accept any new languages, this should not happen. We would then push all languages out of core over time.

This should be done as a whole refractor, extracting other key languages.

Perhaps, but it depends on project resources (which are extremely limited). What we want to do is not have people wait to have their languages merged. We can just focus on the build system and core.

[marcoscaceres]

We should have this discussion in #1759 - it's off topic here:)

[chenglou]

Alright, submitted a PR ^

[gmmorris]

Hey,
Just to clarify regarding:

As the project maintainers would not accept any new languages, this should not happen. We would then push all languages out of core over time.

I had assumed that's the case :) I just want to prevent developers spending time and submitting a PR just to be told it already exists as a plug-in.

Communicating this well is important, but definitely not a blocker. :)

(sorry for the late response, I've been on leave.)

[marcoscaceres]

Agree. We need to look at how Babel and, say, oh-my-zsh, have dealt with this on a communication level.

Having said that, the only way we can make this happen is to find a few more folks willing to help. It’s definitely doable, but might have to be an xmas project.

[gmmorris]

I'd actually be very happy to help... In between leading a 12 person team and pursuing my own OSS I'm kind of stretched, but who isn't. 🤣

[marcoscaceres]

@gmmorris, I'm in the same boat so totally understand! if you could help me a little bit with the PR backlog, it would be greatly appreciated (e.g,. one or two reviews per week would already be a massive help). Or even just helping close old bugs, etc.

The main thing the project needs is a new build system and to get rid of some of the old dependencies that are being flagged as insecure.

[gmmorris]

Happy to help 😁
I'll try and find the time.

[joshgoebel]

@chenglou Could your changes be merged back into mainline now, or do you still feel they need to live outside? The original reason for having your own version was "in order to iterate on it faster"... but I'd assume that day-to-day iteration once you got it into a good place would be slower, no?

I'd like to decide what we're doing here and close this issue.

The choices I see:

1. Merge the external changes back into core and maintain them there, and deprecate the external repo.
2. Remove reasonml from core in favor of the improved 3rd party module
3. Do nothing (other than close this issue), letting core be old and kind of junky while the 3rd party module is much better.

I'm not a huge fan of choice 3 though.

CC @egor-rogov

[joshgoebel]

@chenglou Ping.

[joshgoebel]

Closing this issue due to lack of inactivity. If @chenglou doesn't really want it merged back into core and we have no one else to champion doing so... The lack of response isn't necessarily a great sign regarding the maintenance of the module outside core either, so removing it from core also seems premature. I guess this is just dead for now, with no real resolution either way.

If anyone has interest or wants to champion this we can always revisit. Closing for now.