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

on_attach is being called twice when separate_diagnostic_server is true #208

Closed
b0o opened this issue Jan 1, 2024 · 11 comments · Fixed by #213
Closed

on_attach is being called twice when separate_diagnostic_server is true #208

b0o opened this issue Jan 1, 2024 · 11 comments · Fixed by #213

Comments

@b0o
Copy link

b0o commented Jan 1, 2024

Is this intentional? This is causing issues with twoslash-queries, causing duplicate virtual text. Here is my full config:

require('typescript-tools').setup {
  on_attach = function(client, bufnr)
    require('twoslash-queries').attach(client, bufnr)
    require('user.lsp').on_attach(client, bufnr)
  end,
  settings = {
    separate_diagnostic_server = true,
    publish_diagnostic_on = 'insert_leave',
    tsserver_file_preferences = {
      includeCompletionsForModuleExports = true,

      -- inlay hints
      includeInlayParameterNameHints = 'literals',
      includeInlayParameterNameHintsWhenArgumentMatchesName = false,
      includeInlayFunctionParameterTypeHints = true,
      includeInlayVariableTypeHints = true,
      includeInlayVariableTypeHintsWhenTypeMatchesName = false,
      includeInlayPropertyDeclarationTypeHints = true,
      includeInlayFunctionLikeReturnTypeHints = true,
      includeInlayEnumMemberValueHints = true,
    },
    tsserver_format_options = {
      allowIncompleteCompletions = false,
      allowRenameOfImportPath = false,
    },
  },
}
@pmizio
Copy link
Owner

pmizio commented Jan 2, 2024

Probably it isn't. We spawn 2 tsservers in this mode indeed, but still one lsp should be attached. I need to check this.

@pmizio
Copy link
Owner

pmizio commented Jan 9, 2024

@b0o it should be fixed in connected pr, can you check it?

@b0o
Copy link
Author

b0o commented Jan 10, 2024

@pmizio Yes, that fixed it. Thank you!

@pmizio
Copy link
Owner

pmizio commented Jan 10, 2024

@b0o merged into master! Thanks for checking it up.

@chaozwn
Copy link

chaozwn commented Jan 13, 2024

Probably it isn't. We spawn 2 tsservers in this mode indeed, but still one lsp should be attached. I need to check this.

I think it might be because they didn't disable the tsserver.

@b0o
Copy link
Author

b0o commented Jan 13, 2024

I think it might be because they didn't disable the tsserver

Nope, I only had typescript-tools enabled.

@chaozwn
Copy link

chaozwn commented Jan 13, 2024

I think it might be because they didn't disable the tsserver

Nope, I only had typescript-tools enabled.

The latest fix PR is problematic; you can try using it to open a React project. Then attempt to rename a variable, and open two files consecutively. You will notice that only the first file is affected.

@b0o
Copy link
Author

b0o commented Jan 13, 2024

Hmm, I don't have the same issue here. Opening two files and renaming a variable shared between them works as expected.

@b0o
Copy link
Author

b0o commented Jan 13, 2024

@chaozwn does #218 fix this for you?

@chaozwn
Copy link

chaozwn commented Jan 13, 2024

@chaozwn does #218 fix this for you?

I'll test it out.

@chaozwn
Copy link

chaozwn commented Jan 13, 2024

@chaozwn does #218 fix this for you?

works well. wait merge.

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