search

ebkalderon / tower-lsp

Language Server Protocol implementation written in Rust
Apache License 2.0
1k stars 55 forks source link

Consider merging with lspower and maintaining a single code base #308

Closed silvanshade closed 2 years ago

silvanshade commented 2 years ago

Hi @ebkalderon, I wanted to start a discussion about merging some of the changes I made when lspower was forked and then deprecating that project so we can maintain a single code base. (I think you pinged me about this but I don't see where now so I'll just create this new issue).

I think the only significant changes since you've been updating tower-lsp again are:

  1. The use of async-codec-lite in order to facilitate wasm targets re: https://github.com/ebkalderon/tower-lsp/issues/187
  2. The use of httparse and twoway instead of nom. This is more efficient and reduces the number of dependencies.

A lot of the rest of the changes are either bug fixes or updates you've already made or stylistic changes which aren't important.

I could work on PRs for the above two and, deprecate lspower, and then help to maintain tower-lsp going forward if that is something that interests you.

ebkalderon commented 2 years ago

Hello again, @silvanshade! It's great to hear from you again. Thanks so much for maintaining lspower in my absence and submitting excellent improvements to the codebase all the while.

Sure, I would be happy to merge in those two changes into tower-lsp and resume the project in a more full capacity, if you're okay with that! Although small note: the twoway crate is technically considered deprecated today and advises users to go for memchr instead. This should still be fewer dependencies than relying on nom, though, either way. I'd also be happy to grant you some rights to the repository, if you're interested, considering your excellent contributions so far.

I'm looking forward to rolling out the custom client-to-server request approach described in https://github.com/ebkalderon/tower-lsp/issues/256#issuecomment-860024369, as well as addressing #284 long-term (I think I may have a vague idea of how to proceed and will update the issue shortly), and in light of this, I think it would be wonderful if the two codebases were consolidated together.

Feel free to get started on those PRs and we can review them together! And please freely ping me if you have further questions or suggestions as well. :heart:

ebkalderon commented 2 years ago

Just merged in the changes, @silvanshade! What would you say to receiving collaborator access to this repo, adding you as an author to the Cargo.toml, and possibly adopting the lspower name unilaterally to try to unify the user bases of both crates?

silvanshade commented 2 years ago

@ebkalderon thanks for the follow up! That all sounds good to me. I'm not entirely sure if it would be better to use lspower or just keep the name tower-lsp. I don't really have a strong opinion about it either way so I guess it's up to you but it's fine with me if you think that would be best.

silvanshade commented 2 years ago

So one minor point in favor of keeping the tower-lsp name is that I already created a cancellation token API for lspower but it's not as nice as the one you described recently. So moving to that would be a breaking change I guess. I kind of doubt anyone is actually using it but still.

ebkalderon commented 2 years ago

That's certainly a fair point. I wouldn't want to cause confusion for existing lspower and tower-lsp users. I guess we could shift focus towards #207 and choose a new name instead, once the API is finally stabilized.

ebkalderon commented 2 years ago

Now that version 0.16.0 is up on crates.io, this task is effectively complete. :tada: I've personally added you, @silvanshade, as a co-author of the tower-lsp crate in the Cargo.toml. If you would like collaborator access to the repository going forward, please feel free to reach out to me personally. Thanks again for your great co-maintainership and contributions so far!