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: Don't send empty completion command #76790

Merged
merged 1 commit into from
May 9, 2023
Merged

LSP: Don't send empty completion command #76790

merged 1 commit into from
May 9, 2023

Conversation

achernik
Copy link
Contributor

@achernik achernik commented May 6, 2023

Fixes #76789
makes it so that the server doesn't send an empty command, so you get a response like this:

{
  "sortText": "",
  "preselect": null,
  "label": "collision_polygon",
  "kind": 6,
  "insertText": "collision_polygon",
  "filterText": "",
  "documentation": {
    "value": "\tvar collision_polygon: PackedVector2Array\n\nDefined in [res://test_script.gd](file:///Users/achernik/projects/test_lsp/test_script.gd)",
    "kind": "markdown"
  },
  "detail": "",
  "deprecated": null,
  "data": {
    "textDocument": {
      "uri": "file:///Users/achernik/projects/test_lsp/test_script.gd"
    },
    "position": {
      "line": 48,
      "character": 7
    },
    "context": null
+  }
-  },
-  "command": {
-    "title": "",
-    "command": ""
  }
}

@achernik achernik requested a review from a team as a code owner May 6, 2023 19:58
@YeldhamDev YeldhamDev added this to the 4.x milestone May 7, 2023
@achernik
Copy link
Contributor Author

achernik commented May 9, 2023

Should be cherry-pickable for 4.0.x

@akien-mga akien-mga modified the milestones: 4.x, 4.1 May 9, 2023
@akien-mga akien-mga merged commit c2ba89d into godotengine:master May 9, 2023
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

Note for future PRs: We recommend making PRs from a dedicated branch, instead of your master, so that you can easily create new branches off master to submit multiple PRs in parallel. This one was merged fast, but it's not rare for PRs to take weeks or months to be merged, and you typically don't want to have your fork's master branch locked because of that.

@achernik
Copy link
Contributor Author

achernik commented May 9, 2023

Thanks! And congrats for your first merged Godot contribution 🎉

Thank you! It was really easy to setup and build the project, kudos to the team 👍

Note for future PRs: We recommend making PRs from a dedicated branch

Thanks for the tip! Will do that from now on.

@akien-mga
Copy link
Member

Cherry-picked for 4.0.3.

@akien-mga akien-mga changed the title LSP: don't send empty completion command LSP: Don't send empty completion command May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LSP - completionItem/resolve: ComplitionItem structure returns a Command with empty ID
3 participants