Skip to content

fix: remove unnecessary "bin/start-stdio" script #20

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

Merged
merged 1 commit into from
Dec 29, 2024

Conversation

synic
Copy link
Contributor

@synic synic commented Dec 29, 2024

I think I figured out how it's supposed to work without adding additional scripts or worrying about passed arguments.

The mason-registry submission is here: mason-org/mason-registry#8368

Once refactorex is installed with mason, it can be configured using the following code:

require("lspconfig.configs").refactorex = {
  default_config = {
    cmd = { vim.fn.stdpath("data") .. "/mason/bin/refactorex", "--stdio" },
    filetypes = { "elixir" },
    root_dir = function(fname)
      return require("lspconfig").util.root_pattern("mix.exs")(fname)
    end,
    settings = {},
    capabilities = {
      textDocumentSync = {
        openClose = true,
        change = 1,
        save = { includeText = true },
      },
      codeActionProvider = {
        resolveProvider = true,
      },
      renameProvider = {
        prepareProvider = true,
      },
    },
  },
}

require("lspconfig").refactorex.setup({})

As you can see, the cmd has the --stdio argument. Mason's job is mainly just to install the language server, not to actually say what will be done with it.

For maximum ease, once it is accepted in Mason, we will need to also submit it to https://github.com/williamboman/mason-lspconfig.nvim, similar to how nextls has done it here:
https://github.com/williamboman/mason-lspconfig.nvim/blob/main/lua/mason-lspconfig/server_configurations/nextls/init.lua

return function()
  return {
    cmd = { "nextls", "--stdio" },
  }
end

So, mason is just to install the language server, mason-lspconfig is what provides the easy-to-use lsp configuration. Until it's accepted in mason-lspconfig, your friends can use the snippet above to get it working (once it's accepted to mason itself).

Until it's accepted in mason, they can just use this plugin: https://github.com/synic/refactorex.nvim - it just downloads the refactorex source code to a local directory and runs it from there. Seems to work just fine.

Sorry about the runaround with bin/start-stdio - learning all of this as I go ;-)

I think I figured out how it's supposed to work without adding
additional scripts or worrying about passed arguments.

The mason-registry submission is here: mason-org/mason-registry#8368

Once refactorex is installed with mason, it can be configured using the
following code:

```lua
require("lspconfig.configs").refactorex = {
  default_config = {
    cmd = { vim.fn.stdpath("data") .. "/mason/bin/refactorex", "--stdio" },
    filetypes = { "elixir" },
    root_dir = function(fname)
      return require("lspconfig").util.root_pattern("mix.exs")(fname)
    end,
    settings = {},
    capabilities = {
      textDocumentSync = {
        openClose = true,
        change = 1,
        save = { includeText = true },
      },
      codeActionProvider = {
        resolveProvider = true,
      },
      renameProvider = {
        prepareProvider = true,
      },
    },
  },
}

require("lspconfig").refactorex.setup({})
```

As you can see, the `cmd` has the `--stdio` argument. Mason's job is
mainly just to install the language server, not to actually say what
will be done with it.

For maximum ease, once it is accepted in Mason, we will need to also
submit it to https://github.com/williamboman/mason-lspconfig.nvim,
similar to how nextls has done it here:
https://github.com/williamboman/mason-lspconfig.nvim/blob/main/lua/mason-lspconfig/server_configurations/nextls/init.lua

```lua
return function()
  return {
    cmd = { "nextls", "--stdio" },
  }
end
```

So, mason is just to install the language server, mason-lspconfig is
what provides the easy-to-use lsp configuration. Until it's accepted in
`mason-lspconfig`, your friends can use the snippet above to get it
working (once it's accepted to mason itself).

Until it's accepted in mason, they can just use this plugin:
https://github.com/synic/refactorex.nvim - it just downloads the
refactorex source code to a local directory and runs it from there.
Seems to work just fine.
@gp-pereira
Copy link
Owner

gp-pereira commented Dec 29, 2024

Wow, that's amazing, nice job! The integration is coming out clean.

Since you seem so interested in making all of this work nicely, I'd like to ask if you could move your plugin to the extensions/neovim folder in this repo along side instructions on how to set it up. After that, I'd gladly make you a project maintener so you can easily work on this integration. Also, I wouldn't mind if the integration/plugin evolves over time as we figure out the best way to do it.

I don't intend to switch to neovim (at least not soon) so having someone that actually uses it would definitely make the different in providing the same experience that I try to bring to the vs code.

Anyway, thank you again. I really apreciate all the effort you put into making it work for neovim users!

@gp-pereira gp-pereira merged commit 6941bfc into gp-pereira:main Dec 29, 2024
@synic
Copy link
Contributor Author

synic commented Dec 29, 2024

I could move it to the main repository, but there are some requirements that may or may not make it a bit less desirable.

The main bulk of the code can be in extensions/neovim, but to make it easy to set up with the popular plugin managers (like lazy.nvim, there needs to be at least a lua/refactorex.lua in the root as an entry point. This is just how neovim looks up lua code.

Now, it's possible to tell it to look in another location (for example, extensions/neovim/lua), however, this would need to be done by the user, it cannot be done by the plugin itself, due to a chicken and egg situation (can't load the lua code to tell neovim to use a different directory if the lua code can't be loaded)

So, for example, with a lua/ directory in the root, your lazy.nvim configuration for refactorex would look something like this:

{
  "gp-pereira/refactorex",
  ft = "elixir",
  config = true,
}

If there's not a lua directory, it would look more like this:

{
  "gp-pereira/refactorex",
  ft = "elixir",
  config = function()
    vim.opt.runtimepath:append(vim.fn.stdpath("data") .. "lazy/refactorex/extensions/neovim")
    require("refactorex").setup({})
  end,
}

which, obviously could just be mentioned in the docs, it's just kinda ugly and non-standard

@synic
Copy link
Contributor Author

synic commented Dec 30, 2024

Yeah, I tried a bunch of stuff. While it can go in extensions/neovim with this config:

{
  "gp-pereira/refactorex",
  ft = "elixir",
  config = function(plugin)
    vim.opt.rtp:append(plugin.dir)
    require("refactorex").setup()
  end,
}

.. which isn't too bad, but because there's no docs/refactorex.txt (which is where neovim looks for docs) when the plugin is initialized with lazy.nvim, lazy.nvim just creates one out of the README.md, which doesn't have much to do with neovim. When the user types :help refactorex, they see everything in README.md

For the very best, most standard neovim experience, there needs to be:

  1. lua/refactorex.lua
  2. doc/refactorex.txt - but this would be neovim specific, which is less than ideal.

I think for the time being, having a separate repository is best, and that can be archived when mason adds it.

@gp-pereira
Copy link
Owner

gp-pereira commented Dec 30, 2024

100% agree! Let's leave them apart for now. But I still think it would be cool if any Neovim user that lands here has some way to see your work. Maybe a redirect on the README.md to some other markdown with instructions by editor? The Neovim ones would then redirect your repo. How do you think we can do something like that?

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 this pull request may close these issues.

2 participants