rescript-lang / tree-sitter-rescript

ReScript parser for Tree-Sitter
MIT License
50 stars 15 forks source link

Future maintenance of this package #238

Closed Emilios1995 closed 7 months ago

Emilios1995 commented 8 months ago

I acknowledge and am deeply grateful for the work @nkrkv put into this parser.

However, it hasn't been updated for a long time, even with multiple PRs open. I think that rescript deserves proper support in Neovim and other modern editors, and I also think there's enough members of the community that care about this; we just need to get organized. With the advent of newer editors such as Zed and Helix, this module will become even more important.

It would be ideal to form a group of maintainers, get the repo into a stable state, and pitch the Rescript lang maintainers to move this parser into the rescript-lang umbrella GitHub org. Then, we should bundle the parser info into the official Neovim tree-sitter module so that folks can simply TSInstall rescript

I took the liberty of merging many of the currently open PRs into my own fork as a starting point, but again, the goal is to get it under the umbrella organization.

As a disclaimer, I'm by no means a tree-sitter expert, but I care enough about the problem to help out. For example, I'm still not sure why it's necessary to handle newlines in the external scanner instead of relaying on the default lexing flow with the correct precedence rules (among other questions). But even if we need a refactor, it would be good to get into a stable shape first.

So, what do you all think? How do you feel about the overall state of this parser? Could it be refactored and simplified? Would you like to help maintain and move the repo to the rescript-lang org?

Of course @nkrkv, if you still mean to maintain this and are just on a temporary hiatus, please let us know.

CC @aspeddro @cristianoc @zweimach @TheFedaikin

aspeddro commented 8 months ago

Thank you for bringing this discussion.

The parser needs to be updated and improved. The last time I used the parser in Neovim I noticed some bottlenecks when editing jsx in large files.

As a disclaimer, I'm by no means a tree-sitter expert, but I care enough about the problem to help out. For example, I'm still not sure why it's necessary to handle newlines in the external scanner instead of relaying on the default lexing flow with the correct precedence rules (among other questions). But even if we need a refactor, it would be good to get into a stable shape first.

I think when we have something that can appear anywhere and tree-sitter could not determine the end of the rule, it's best to deal with it in the scanner. That's what I did when moving the decorators to the scanner. If we are going to do this in grammar.js we have to add the $.decorator rule in several places introducing conflicts.

nkrkv commented 8 months ago

Hey, gentlemen! Sorry for abandoning the package. Indeed I’m far from ReScript last year or so, things changed in my life and I’ve silenced many notification channels to keep business manageable. Well, it happened that this repo was silenced as well :grimacing:

With the great pleasure I would transfer ownership of the repo to ReScript community. @aspeddro made so many contributions, that I’m :100: sure, the package is in good hands. Let’s discuss the technical details of ownership transfer? Whom should I transfer and when?

P.S>Thanks Dmitry for reaching me out and pointing to the issue.

fhammerschmidt commented 8 months ago

Please transfer it to https://github.com/rescript-lang

I'll then ensure that @aspeddro,and maybe also @Emilios1995, have write access.

nkrkv commented 8 months ago

image

Well, I need permissions to create public repos in “rescript-lang” org to complete it. I see two options:

  1. Someone provides me the permission (perhaps temporarily, just for the transfer duration)
  2. I transfer to @aspeddro first, then he transfers to “rescript-lang”

The second option looks lighter, I just wonder if GitHub will like such a chain in terms of URL redirects and stuff :thinking: It should work, but maybe someone foresees some problems here?

fhammerschmidt commented 8 months ago

As @aspeddro is not yet a member of rescript-lang, maybe transfer it to me and I will move it over.

nkrkv commented 8 months ago

@fhammerschmidt I’ve sent a transfer request for this repo and the second one for https://github.com/nkrkv/nvim-treesitter-rescript

fhammerschmidt commented 8 months ago

Did it go well? https://github.com/rescript-lang/tree-sitter-rescript https://github.com/rescript-lang/nvim-treesitter-rescript

At least issues and PRs have been preserved. Redirects seem to work as well.

Added @aspeddro as a maintainer to both projects.

aspeddro commented 8 months ago

@nkrkv Thank you very much for creating this parser and transferring it to the community.

Emilios1995 commented 8 months ago

@nkrkv No need to apologize, thanks a lot for your work. And thanks for doing the move, @fhammerschmidt. I think we won't need the second repo anymore, but we'll see.

@aspeddro I made a PR to merge my fork's main branch, in which I integrated the open PRs that didn't have any problems. It also updates the instructions to point to the new location. If you prefer you can also merge the PRs one by one, I didn't have to fix many conflicts anyway.

nkrkv commented 8 months ago

@fhammerschmidt @aspeddro @Emilios1995 :bow: bless you for all the work done for ReScript and giving the tree-sitter-rescript the new home!