Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lsp-dart-line-length stopped working on flutter 2.5 #120

Closed
sysint64 opened this issue Sep 13, 2021 · 24 comments
Closed

lsp-dart-line-length stopped working on flutter 2.5 #120

sysint64 opened this issue Sep 13, 2021 · 24 comments
Labels
bug Something isn't working

Comments

@sysint64
Copy link

Describe the bug
lsp-dart-line-length stopped working on flutter 2.5. On version 2.2 it works fine.

To Reproduce
Install flutter 2.5 and set the line length to 100 symbols;

(setq lsp-dart-line-length 100)

format buffer with lsp-format-buffer

@ericdallo
Copy link
Member

I'll take a look at this, probably something related with dart sdk bump

@ericdallo
Copy link
Member

ericdallo commented Sep 13, 2021

yeah, I confirmed the issue happens on dart 2.14.0 but not on dart 2.13.1.
@DanTup do you know if there was introduced on dart-sdk any regression or new variable to control the line length during formatting? I checked Dart-Code but I didn't see any changes about that

@ericdallo
Copy link
Member

This is how lsp-dart is sending the workspace/configuration response after changing the default from 80 -> 100:

[Trace - 10:24:29 AM] Sending response 'workspace/configuration - (1)'. Processing request took 0ms
Params: {
  "jsonrpc": "2.0",
  "id": 1,
  "result": [
    {
      "dart": {
        "enableSdkFormatter": true,
        "completeFunctionCalls": true,
        "showTodos": null,
        "lineLength": 100
      }
    }
  ]
}

@ericdallo ericdallo added the bug Something isn't working label Sep 13, 2021
@DanTup
Copy link

DanTup commented Sep 13, 2021

There was a change recently to support per-workspace config. Are you able to capture the traffic (or use --instrumentation-log) for a session where this doesn't work for me to check?

It should be falling back to the original workspace-level config when there's no folder config, but I wonder if something is going wrong.

@DanTup
Copy link

DanTup commented Sep 13, 2021

@ericdallo do you have the request that the server sent to you that this responds to?

@ericdallo
Copy link
Member

sure, the server request was:

[Trace - 10:24:29 AM] Received request 'workspace/configuration - (1).
Params: {
  "items": [
    {
      "section": "dart"
    }
  ]
}

@DanTup
Copy link

DanTup commented Sep 14, 2021

@ericdallo the "dart" in your response looks suspect. When I log from VS Code, I see a request from the server like this:

{
	"id": 1,
	"method": "workspace/configuration",
	"params": {
		"items": [
			{
				"section": "dart"
			}
		]
	},
	"jsonrpc": "2.0"
}

And VS Code responds like this (note there's no "dart" anywhere):

{
	"jsonrpc": "2.0",
	"id": 1,
	"result": [
		{
			"lineLength": 160,
			// ...
		}
	]
}

Although the LSP spec doesn't seem to be very clear about this.

@ericdallo
Copy link
Member

@yyoncho can you help understand what is the probably expected behavior?
Even so, the best would be to fix on lsp-dart/lsp-mode somehow otherwise we would need to wait for another dart-sdk update + flutter update which may take some time

@DanTup
Copy link

DanTup commented Sep 14, 2021

I don't think the spec is very clear, but there is some discussion in microsoft/language-server-protocol#972 which I think matches the Dart/VS Code stuff above - that is if a server asks for "dart" configuration, the returned object is already scoped to "dart" and should not have all the values wrapped inside another "dart".

If it seems like there are many editors/servers doing the opposite, we should probably ping on microsoft/language-server-protocol#972 and see if we can get concrete clarification (and if that means changing the server and/or VS Code LSP client, I'm happy to do that).

@ericdallo
Copy link
Member

thank you @DanTup, that makes sense to me indeed.
@yyoncho do you think we can change the lsp-mode response unwrapping the returned item? I don't know any other server that uses workspace/configuration so I'm not sure if a lot of servers would be affected though

@yyoncho
Copy link
Member

yyoncho commented Oct 4, 2021

@ericdallo makes sense

@ericdallo
Copy link
Member

I'm not sure why, but testing now, I can't get lsp-mode to return the configuration wrapped by a dart map, testing with both dart SDK version (stable and dev), I'm getting the same response as vscode, but the dev one still is not working.

with dart SDK 2.13.1

[Trace - 12:20:07 PM] Received request 'workspace/configuration - (1).
Params: {
  "items": [
    {
      "section": "dart"
    }
  ]
}


[Trace - 12:20:07 PM] Sending response 'workspace/configuration - (1)'. Processing request took 0ms
Params: {
  "jsonrpc": "2.0",
  "id": 1,
  "result": [
    {
      "enableSdkFormatter": true,
      "completeFunctionCalls": true,
      "showTodos": null,
      "lineLength": 120
    }
  ]
}

with dart SDK 2.15.0

[Trace - 12:26:48 PM] Received request 'workspace/configuration - (1).
Params: {
  "items": [
    {
      "scopeUri": "file:///home/greg/dev/flutter_sample",
      "section": "dart"
    },
    {
      "section": "dart"
    }
  ]
}


[Trace - 12:26:48 PM] Sending response 'workspace/configuration - (1)'. Processing request took 0ms
Params: {
  "jsonrpc": "2.0",
  "id": 1,
  "result": [
    {
      "enableSdkFormatter": true,
      "completeFunctionCalls": true,
      "showTodos": null,
      "lineLength": 120
    }
  ]
}

It seems to me that the client is returning correctly, any thoughts @yyoncho ?

@yyoncho
Copy link
Member

yyoncho commented Oct 9, 2021

We do unwrap and return the result by position (as you can see it is returning a vector).

@ericdallo
Copy link
Member

So can we say it's something that changed on the newest dart SDK @DanTup ?

@DanTup
Copy link

DanTup commented Oct 9, 2021

@ericdallo the last request/response posted above don't look right to me. The request is asking for two items:

"items": [
    {
      "scopeUri": "file:///home/greg/dev/flutter_sample",
      "section": "dart"
    },
    {
      "section": "dart"
    }
  ]

But the response contains only one:

  "result": [
    {
      "enableSdkFormatter": true,
      "completeFunctionCalls": true,
      "showTodos": null,
      "lineLength": 120
    }
  ]

The items in the resulting array need to match up with the request:

https://microsoft.github.io/language-server-protocol/specifications/specification-current/#workspace_configuration

The order of the returned configuration settings correspond to the order of the passed ConfigurationItems (e.g. the first item in the response is the result for the first configuration item in the params).

If the client can’t provide a configuration setting for a given scope then null needs to be present in the returned array.

The server will ignore the response if the number of items don't match the request:

https://github.com/dart-lang/sdk/blob/e995cb5f7cd67d39c1ee4bdbe95c8241db36725f/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart#L241

(It is a result of a Dart SDK change that we send a request for config for multiple items and didn't before, but I think the fix needs to be here, the change in the SDK doesn't seem like a bug - perhaps an error could be reported somewhere better, although I suspect this on't be a common bug once fixed here).

@ericdallo
Copy link
Member

I see, it makes sense to me indeed!
So we probably need to fix lsp-mode to return the matching items, right @yyoncho ?

@yyoncho
Copy link
Member

yyoncho commented Oct 9, 2021

The issue is in lsp-dart - it should not define custom handler lsp-dart--configuration but it should use lsp-register-custom-settings to register dart settings and then let lsp-mode build the correct response.

@ericdallo
Copy link
Member

Confirmed it works indeed :)
Thank you @yyoncho and @DanTup for the help on this!

@ericdallo
Copy link
Member

Available on next 1.19.5 release @sysint64

@laynor
Copy link

laynor commented Dec 9, 2021

I'm still stuck with non-working lsp-dart-line-length setting, even though my lsp-dart version is 1.20.0.
How can I fix it?

@ericdallo
Copy link
Member

ericdallo commented Dec 9, 2021

What's your lsp-dart-version command output @laynor ?

@laynor
Copy link

laynor commented Dec 11, 2021

@ericdallo Here it is

[LSP Dart] 1.20.0 at 2021.12.12 @ Emacs 28.0.50
[Dart SDK] Dart SDK version: 2.15.0 (stable) (Fri Dec 3 14:23:23 2021 +0100) on "macos_x64"

[Flutter SDK] /Users/ale/local/opt/flutter
[Flutter project] true
[Project entrypoint] Not found

@ericdallo
Copy link
Member

@laynor I just tested with latest flutter (2.8.0, dart 2.15.0) and latest lsp-dart 1.20.1 and it works properly after setting: (setq lsp-dart-line-length 100), could you bump lsp-dart to latest?

@laynor
Copy link

laynor commented Dec 17, 2021

@ericdallo Can confirm it's working 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants