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

Dynamic registrations of source code actions are not supported in "on_save" trigger #2201

Closed
james2doyle opened this issue Feb 17, 2023 · 10 comments · Fixed by #2202
Closed

Comments

@james2doyle
Copy link
Contributor

Describe the bug

The lsp_format_on_save and lsp_code_actions_on_save do not seem to be supported by code actions that are dynamically registered. The code actions work fine when you run them manually.

To Reproduce

  1. Install latest Sublime Text 4 (4147)
  2. Install latest Sublime LSP (1.22.0)
  3. Install latest Dart (SDK version: 3.0.0-239.0.dev)
  4. Install latest LSP-Dart
  5. Enable lsp_format_on_save and lsp_code_actions_on_save for source.organizeImports, source.fixAll, and source.sortMembers
  6. Save a Dart file with illogical imports or formatting issues
  7. Observe the file remains unchanged after save

Expected behavior
The source actions are run on save and the file is updated

Screenshots
If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

  • macOS 13.1
  • Sublime Text 4 (4147)
  • Sublime LSP (1.22.0)
  • Dart (Dart SDK version: 3.0.0-239.0.dev)

Additional context
I originally filed this issue on the Dart SDK repo as that is where the language server is. I observed that on startup the Dart LSP was not registering any "source" capabilities. It started with the following:

Dart: Supported execute commands: ['edit.sortMembers', 'edit.organizeImports', 'edit.fixAll', 'edit.sendWorkspaceEdit', 'refactor.perform', 'move_top_level_to_file']

So I thought there might be an issue with the Dart LSP itself. But it turns out that the Dart LSP registers their actions dynamically.

After enabling the Dart LSP log, it is clearly registering the capabilities. It is also registering the "autoTriggered" events. But the file is not being updated:

1676656755429:Res:{
  "id": 3,
  "jsonrpc": "2.0",
  "result": [
    {
      "command": {
        "arguments": [
          {
            "path": "/Users/jamesdoyle/Git/flutter-explorations/ed_team_app/lib/src/team_feature/team_member_list_view.dart",
            "autoTriggered": true
          }
        ],
        "command": "edit.organizeImports",
        "title": "Organize Imports"
      },
      "kind": "source.organizeImports",
      "title": "Organize Imports"
    },
    {
      "command": {
        "arguments": [
          {
            "path": "/Users/jamesdoyle/Git/flutter-explorations/ed_team_app/lib/src/team_feature/team_member_list_view.dart",
            "autoTriggered": true
          }
        ],
        "command": "edit.fixAll",
        "title": "Fix All"
      },
      "kind": "source.fixAll",
      "title": "Fix All"
    }
  ]
}
1676656756600:Watch:<unknown>:/Users/jamesdoyle/Git/flutter-explorations/ed_team_app/lib/src/team_feature/team_member_list_view.dart:modify

So the actions work when running manually, the events are showing in the log, but the file is never edited after the save.

@rchl
Copy link
Member

rchl commented Feb 17, 2023

Can you provide more complete log from LSP: Toggle Log Panel? From start of the server up to triggering a save.

@rchl
Copy link
Member

rchl commented Feb 17, 2023

Never mind. I've read the thread in the Dart repo and explanation from the maintainer makes everything clear. As guessed there, we don't consider dynamic registrations of code action provider, or at least not consider the kinds reported.

@james2doyle james2doyle changed the title Dynamic restrations of source code actions are not supported in "on_save" trigger Dynamic registrations of source code actions are not supported in "on_save" trigger Feb 17, 2023
@james2doyle
Copy link
Contributor Author

Ah ok. Do you consider this a bug then or a feature request?

@rchl
Copy link
Member

rchl commented Feb 17, 2023

I think more like a bug but does it make a difference? :)

@james2doyle
Copy link
Contributor Author

Haha no I guess not. Just thought it might affect the priority of when it gets looked at. Thanks for looking into it

@rchl
Copy link
Member

rchl commented Feb 17, 2023

@DanTup I see some weird behavior that might be a bug. The "fix all" action doesn't fix a missing semicolon as I would expect it to. The "Insert ';'` code action does:

Screen.Recording.2023-02-17.at.21.16.57.mov

This also affects code actions on save but I think the root issue is with just the "fix all" not working in this case.

@rchl
Copy link
Member

rchl commented Feb 17, 2023

Server responds with empty response to workspace/executeCommand:

{
  "arguments": [
    {
      "path": "/usr/local/workspace/sublime-packages/LSP/plugin/core/test.dart"
    }
  ],
  "command": "edit.fixAll"
}

@DanTup
Copy link

DanTup commented Feb 17, 2023

@rchl currently the "Fix All" command for Dart does not support all individual fixes. Some of them require some review/tweaking to ensure the edits they produce can be cleanly merged with edits from other fixes.

If you want a fix for testing, running Fix All on the following code should remove both as Strings:

var a = '' as String;
var b = '' as String;

@rchl
Copy link
Member

rchl commented Feb 17, 2023

Thanks, that works with the fix.

@DanTup
Copy link

DanTup commented Feb 18, 2023

Great, thanks for looking at this so quickly! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants