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

Feature Request: itemDefaults for completions to be combined with completion item #1338

Closed
rchiodo opened this issue Oct 11, 2023 · 2 comments

Comments

@rchiodo
Copy link
Contributor

rchiodo commented Oct 11, 2023

In Pylance, we return a lot of duplicate data in our completion list. We'd like to use the new itemDefaults for completions.

However, some of the data is not duplicated. Only parts of it. But if we send back itemDefaults, they're not combined with the data on each item.

Here's an example completion result:

{
    "itemDefaults": {
         "data": {
              filePath: "root/test.py",
              position: { line: 0, col: 1 },
              workspacePath: "root"
         }
    }
    items: [
         {
             label: "foo",
             kind: 6
             data: {
                  symbolLabel: "foo" 
             }
         },
         {
             label: "bar",
             kind: 6
             data: {
                  symbolLabel: "bar" 
             }
         }
    ]
}

When a resolve happens (if we send the data this way), the filePath, position, and workspacePath are lost.

Resolve completionItem:

{
      label: "foo",
      kind: 6
      data: {
            symbolLabel: "foo"  // No filePath, position, or workspacePath
      }
}

I believe that's because this line here picks one or the other:

const data = item.data ?? defaultData;

Would it be a breaking change to change this line in the protocolConverter.ts to something like this instead?

   const data = {...(defaultData ?? {}), ... (item.data ?? {});

If that's agreeable, I can submit a PR. Thanks.

@rchiodo
Copy link
Contributor Author

rchiodo commented Oct 11, 2023

Or maybe a different change would be to add something to the spec that allows the combination of data. The main goal is to dedupe data on every completionItem.

@dbaeumer
Copy link
Member

Looks like a dup to microsoft/language-server-protocol#1802 for me.

The spec currently states that if a item defines items available in the items default it is a replace operation and not a merge operation. So changing the code is not an option since it could break existing users.

Lets continue the discussion in the other item

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

No branches or pull requests

2 participants