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

feat(ts_ls)!: rename tsserver to ts_ls #3232

Merged
merged 2 commits into from
Sep 5, 2024
Merged

Conversation

ofseed
Copy link
Contributor

@ofseed ofseed commented Jul 12, 2024

tsserver cannot be used as an abbreviation for typescript-language-server, because there is tsserver already and it is completely different from typescript-language-server. This is misleading.

As the README of typescript-language-server says, it's a wrapper of tsserver. This abuse has been around for so many time in lspconfig that people don't realize they are two different things, and are then confused by replacements of typescript-language-server like typescript-tools.nvim and vtsls.

@ofseed ofseed requested a review from glepnir as a code owner July 12, 2024 11:30
Copy link
Contributor

Note that server_configurations.md or server_configurations.txt will be regenerated by the docgen CI process. Edit the Lua source file instead. For details on generating documentation, see: https://github.com/neovim/nvim-lspconfig/blob/master/CONTRIBUTING.md#generating-docs

@ofseed
Copy link
Contributor Author

ofseed commented Jul 12, 2024

Now the server_configuration.md is generated, not manually edited.

@ofseed
Copy link
Contributor Author

ofseed commented Jul 15, 2024

@glepnir Could you please review it?

@glepnir
Copy link
Member

glepnir commented Jul 15, 2024

https://github.com/search?q=lspconfig.tsserver&type=code

quick search it has 5.6k files.. so hasty merge will only result in a lot of complaints. best approach is to popup a notify and keep it available and remove it in the next release.

@ofseed
Copy link
Contributor Author

ofseed commented Jul 15, 2024

best approach is to popup a notify and keep it available and remove it in the next release.

It does, you can see in there, the snippet redirects tsserver to ts_ls and notify tsserver is outdated, will not break users' configuration brutally.

But considering your search result, I will do extra PR for mason.nvim and mason-lspconfig.nvim, and then we can merge them as much as possible at the same time so that most of the users who use mason will not be affected. I will let you know when my PR for mason and mason-lspconfig is ready.

@polyzen
Copy link
Contributor

polyzen commented Jul 16, 2024

Could just wait for microsoft/TypeScript#39459 to be resolved and then do something about it then.

@ofseed
Copy link
Contributor Author

ofseed commented Jul 16, 2024

@polyzen Not that related. There are two points I want to emphasize.

  1. Changing the name will not break users' configurations, lspconfig.tsserver.setup{} is still valid and will only have a warning that tsserver is deprecated suggest using ts_ls instead. In fact, we've already done that for lua_ls, formerly called sumneko_lua, which I think most neovim users will setup no matter what language they are mainly writing. The breaking change should not be that painful.

  2. One of the reasons for creating this PR is to prepare the issue you linked to be resolved. If we do not merge this PR and then merge it after the real tsserver implements LSP, the name tsserver will refer to a different thing after that. That is, tsserver in lspconfig is formerly to be typescript-language-server, but then be the real tsserver. This will cause the meaning of lspconfig.tsserver.setup changed silently, and users' configurations will setup the real tsserver, not typescript-language-server. It should be very difficult for users to find out what happened.

@glepnir
Copy link
Member

glepnir commented Jul 16, 2024

seems like lsp-mode also use ts_ls.. but I'm wondering if this abbreviation is appropriate. How about typescript_ls? ts is a bit ambiguous.

@ofseed
Copy link
Contributor Author

ofseed commented Jul 16, 2024

Keep my recommendation still. ts in the editor area could be tree-sitter or TypeScript, but in the context of Language Server Protocol, I think it will not be confused with tree-sitter, what's more, the former name tsserver using ts as an abbreviation of TypeScript and ts is a well-known abbreviation for its developers. Honestly, the naming of these servers in lspconfig is a mess, foo language server sometimes be foo_ls and sometimes be foo_language_server, I prefer a shorter name if we could name it, otherwise choose the typescript_language_server to make it the same with the repository name.

@justinmk justinmk merged commit bdbc65a into neovim:master Sep 5, 2024
1 check passed
@justinmk
Copy link
Member

justinmk commented Sep 5, 2024

Well reasoned. Thank you

@SannanOfficial
Copy link

SannanOfficial commented Sep 5, 2024

So, umm guys, I now get "tsserver is deprecated, use ts_ls instead" in my neovim after updating. How can I fix this? Do I need to edit some config file and change the name to ts_ls or what? I am asking this because mason doesn't have a separate ts_ls option to install within it.

@justinmk
Copy link
Member

justinmk commented Sep 5, 2024

No idea about mason. Wherever you set "tsserver", change that to "ts_ls".

@ic-768
Copy link

ic-768 commented Sep 5, 2024

So, umm guys, I now get "tsserver is deprecated, use ts_ls instead" in my neovim after updating. How can I fix this? Do I need to edit some config file and change the name to ts_ls or what? I am asking this because mason doesn't have a separate ts_ls option to install within it.

I was also blindsided by this. Here's what I did ( not the full config but you can see how I handle the name change ):

	{
		"williamboman/mason.nvim",
		dependencies = {
			"williamboman/mason-lspconfig.nvim",
			"neovim/nvim-lspconfig",
			"hrsh7th/nvim-cmp",
		},
		config = function()
			require("mason").setup()
			require("mason-lspconfig").setup({
				ensure_installed = {
					"lua_ls",
					"pyright",
					"ruff_lsp",
					"tsserver",
					"eslint",
					"tailwindcss",
					"emmet_language_server",
					"jsonls",
				},
			})

			require("mason-lspconfig").setup()

			-- automatically install ensure_installed servers
			require("mason-lspconfig").setup_handlers({
				-- Will be called for each installed server that doesn't have
				-- a dedicated handler.
				--
				function(server_name) -- default handler (optional)
					-- https://github.com/neovim/nvim-lspconfig/pull/3232
					if server_name == "tsserver" then
						server_name = "ts_ls"
					end
					local capabilities = require("cmp_nvim_lsp").default_capabilities()
					require("lspconfig")[server_name].setup({

						capabilities = capabilities,
					})
				end,
			})

@SannanOfficial
Copy link

@ic-768 Thanks a lot, that was very swift of you, and I will go ahead and give that a try.

@justinmk
Copy link
Member

justinmk commented Sep 5, 2024

Why not just change the "tsserver" item in ensure_installed to "ts_ls" ?

@ic-768
Copy link

ic-768 commented Sep 5, 2024

Why not just change the "tsserver" item in ensure_installed to "ts_ls" ?

Because that would tell mason to install the ts_ls package and no such package exists. It only has tsserver

@SannanOfficial
Copy link

SannanOfficial commented Sep 5, 2024

@ic-768 Hey, I tried to use your code, but I still get the same warning on startup, "tsserver is deprecated, use ts_ls instead", while I can see that tsserver is actually renamed to ts_ls regardless, because it actually says, "Configured servers: { "cssls", "css_variables", "pylsp", "ts_ls" }". Also, there's an error now that says, "/home/sannan/.config/nvim/lua/config/lsp_zero.lua:28: attempt to call field 'setup' (a nil value)" which is quite weird.

Any idea what I might be doing wrong here?

@ic-768
Copy link

ic-768 commented Sep 5, 2024

@SannanOfficial I don't use lsp_zero and I'm not getting any errors, so there might be some conflict with that plugin that needs fixing?

@SannanOfficial
Copy link

SannanOfficial commented Sep 5, 2024

@ic-768 Perhaps, I will look into it. Thanks for the help.

@ofseed
Copy link
Contributor Author

ofseed commented Sep 5, 2024

Originally, I planned to change the package name in mason-registry so that we could merge without many breaking changes, but I noticed a lot of pending pull requests in its repo (maybe the maintainer is busy now) so I did not.

Whenever you have a problem that is difficult to solve, I recommend switching to vtsls. You will have almost the same experience because they are both tsserver under the hood. On top of this, if you installed nvim-vtsls, you could get extra commands that typescript-language-server does not provide.

@SannanOfficial
Copy link

SannanOfficial commented Sep 5, 2024

@ofseed Spent about an hour trying to get this to work for me, but couldn't for now. So I will try and switch to vtsls as you suggested for the time being. Thanks.

Edit: vtsls works, thanks again.

guidovicino added a commit to guidovicino/kickstart-modular.nvim that referenced this pull request Sep 5, 2024
stefanvanburen added a commit to stefanvanburen/dotfiles that referenced this pull request Sep 5, 2024
@aw1875
Copy link

aw1875 commented Sep 5, 2024

if server_name == "tsserver" then
    server_name = "ts_ls"
end

Just came across this but you can also just use a simple one-liner to do the same thing:

server_name = server_name == 'tsserver' and 'ts_ls' or server_name

mettavi added a commit to mettavi/dotfiles that referenced this pull request Sep 5, 2024
uZer added a commit to uZer/.minimics that referenced this pull request Sep 5, 2024
make-github-pseudonymous-again added a commit to make-github-pseudonymous-again/dotfiles-nvim that referenced this pull request Sep 5, 2024
@jepeake
Copy link

jepeake commented Sep 5, 2024

cd ~/.local/share/nvim/lazy/nvim-lspconfig
git checkout HEAD~1

@emanuele-em
Copy link

temporary fix for vim-plug users:

require('mason').setup()
require('mason-lspconfig').setup({
  ensure_installed = {'tsserver','pyright', 'jsonls', 'rust_analyzer'},
  handlers = {
    -- lsp_zero.default_setup,
    function(server_name)
      if server_name == 'tsserver' then
        server_name = 'ts_ls'
      else
        lsp_zero.default_setup(server_name)
      end
    end
  },
})

CoMfUcIoS added a commit to CoMfUcIoS/neovim that referenced this pull request Sep 6, 2024
- Updated the handler for tsserver to ts_ls as per the changes in nvim-lspconfig (neovim/nvim-lspconfig#3232).
yassinebenarbia added a commit to yassinebenarbia/MyNvim that referenced this pull request Sep 6, 2024
for more details check out this PR in the nvim-lspconfig repo neovim/nvim-lspconfig#3232
@neovim neovim deleted a comment from padulkemid Sep 6, 2024
emilioziniades pushed a commit to emilioziniades/dotfiles that referenced this pull request Sep 6, 2024
RainerKuemmerle added a commit to RainerKuemmerle/neovim-lua that referenced this pull request Sep 6, 2024
jollyjerr added a commit to jollyjerr/dotfiles that referenced this pull request Sep 6, 2024
@neovim neovim deleted a comment from cstromquist Sep 6, 2024
@neovim neovim locked as resolved and limited conversation to collaborators Sep 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants