Consider creating a basic lsp client package to share between tools

[parlough]

The protocol is already being generated by https://github.com/dart-lang/sdk/tree/main/pkg/analysis_server/tool/lsp_spec and being placed under third_party/pkg.

Consider cleaning it up, adding a second library entry-point to the package that exposes a basic Dart LSP client (wraps dart language-server, etc), and placing it under pkg.

The client could be used by dart analyze, dart fix, DartPad, perhaps in analysis server tests too, etc?

Since it's understandable to not want to maintain a generic LSP client for all use cases: The scope could be reduced from an all-encompassing LSP package/client to focus on the Dart use case and extensions. If it was useful enough to publish, even in the reduced scope, it could be called package:dart_lsp_client and published tools.dart.dev.

I could definitely use the current protocol generation to create an external package outside the SDK, it just seems a shame to redo or duplicate what the SDK already needs to do.

Edit: To add extra clarification based on comments, I see this as a step towards migrating the above usages to a standardized client implementation based on LSP, increasing standardization towards one protocol.

[bwilkerson]

It isn't clear to me what problem you're trying to solve, or what use case you have in mind.

The client could be used by dart analyze, dart fix, DartPad, perhaps in analysis server tests too, etc?

The analysis_server tests definitely have some minimal client logic in them, but dart analyze and dart fix both use the legacy protocol to talk to the server. I'm not sure about DartPad, but last I knew it was also using the legacy protocol. If we ever decide to convert those tools to using LSP we should definitely consider having a common client implementation, but until then I don't think it would help us.

The only question is whether it would help other parts of the community.

[parlough]

Sorry, I should have added some more context to the issue, I maybe made too many assumptions. I opened this issue a place to have these discussions though, so thanks for asking.

I was exploring converting DartPad to use LSP rather than the legacy protocol, but realized it might be a goal to do the same for the dart commands and I didn't want to unnecessarily repeat work. That way the old client(s) could maybe be discontinued and increase standardization towards one protocol.

Those conversions would be a follow-up to this work. So I guess I see this issue as a place to discuss if those conversions make sense and if this is a good step towards that.

As for being beneficial to the community, I think a standard LSP package and client/server implementations would be very useful. There have been multiple over time, both on the client and server end. I'm not so sure about one limited or focus on a Dart client context, but that's likely much easier to maintain.

[parlough]

I adjusted my original comment a bit to move away from publishing. I think an unpublished, limited client that fulfills the needs of the dart subcommands and potentially DartPad would be a great start.

[bwilkerson]

I was exploring converting DartPad to use LSP rather than the legacy protocol, but realized it might be a goal to do the same for the dart commands and I didn't want to unnecessarily repeat work. That way the old client(s) could maybe be discontinued and increase standardization towards one protocol.

We don't currently have any plans to remove the legacy protocol. While I won't be surprised if our legacy clients eventually support LSP, that's a decision that only they can make. If they do support LSP then we'd probably make plans to deprecate and eventually remove the legacy support, but even if all of our clients supported LSP tomorrow we'd still need to support the legacy protocol for some time in order to continue supporting older versions of those clients.

I don't think it's a priority for us to try to move any of our tools onto LSP until either (a) it's feasible for us to remove support for the legacy protocol or (b) one or more of those tools need a protocol that's only available via LSP (and at the moment it's actually the other way around, fart fix needs a protocol that's only available in legacy mode).

I'm not so sure about one limited or focus on a Dart client context, but that's likely much easier to maintain.

I'm not either. We had a legacy-based client implementation, and as far as I know there was no interest in using it, but that doesn't mean that the demand hasn't changed.

I do agree, though, that if there's interest in the community it would be best if we published our protocol support so that there's a single well supported package that provides it. To what extent it makes sense for us to publish a package to support the use of that protocol on a client I don't know. Writing a few tests doesn't really compare to writing a fully functional client, so it might make more sense for the community to write such a package.

[parlough]

That all makes sense, thanks for the clarification on the current situation :D

It sounds like we're at a point where the status quo is sufficient, but I'll keep this issue open to collect other opinions as well as any community interest/use cases in published protocol support.

(and at the moment it's actually the other way around, dart fix needs a protocol that's only available in legacy mode).

Interesting. What part of the protocol is this? I've mostly worked with the legacy protocol rather than LSP, so I'm curious what piece is missing. I faintly remember Danny landing some iterative "Fix all" support, but perhaps that was per file than for a directory or entire workspace?

[bwilkerson]

What part of the protocol is this?

dart fix uses the experimental edit.bulkFixes protocol.

I faintly remember Danny landing some iterative "Fix all" support, but perhaps that was per file than for a directory or entire workspace?

I believe that that's limited to fixes within a single file, though I might be misremembering. It's certainly possible to implement the functionality as a custom protocol, and it might be closer than I remember, but I don't believe that we currently have the equivalent functionality implemented. If nothing else, I'm fairly sure that the fix all command doesn't handle preview mode.