gbprod / phpactor.nvim

Lua version of the Phpactor vim plugin to take advantage of the latest Neovim features
73 stars 7 forks source link

Allow to use custom utils.path() function #25

Closed foobar1643 closed 10 months ago

foobar1643 commented 10 months ago

Hi, I'm using neotree in netrw mode (with position="current") and when I try to use RPCs with interactive menus (the ones processed by rpc.handle_input_callback()) I get unwanted results when paths to files or directories are involved.

For example, when I attempt to do require('phpactor').rpc('new_class') while in neotree, I get this: 20231220_01h39m48s_grim

It makes sense because that's the name of the current buffer, but not really what needs to be there.

This PR adds a config option in order to allow the plugin to use a custom utils.path() function defined at startup. With these changes I've been able to define my own path resolution function (using neotree API), and now my input looks properly, like this: 20231220_01h41m59s_grim

gbprod commented 10 months ago

Hello, I'm not sure that's the good way to do that. Root detetion is based on lsp root pattern detection. See https://github.com/neovim/nvim-lspconfig and https://github.com/neovim/nvim-lspconfig/blob/master/doc/server_configurations.md#phpactor.

I think it should be better to override lsp-config settings using this option : https://github.com/gbprod/phpactor.nvim#lspconfigoptions

Something like :

  {
     "gbprod/phpactor.nvim",
     lspconfig = {
        enable = true,
        options = {
          root_dir = function(pattern)
             -- You're own root detection
          end,
        },
      },
    },
  },

You can find the phpactor default root detection here : https://github.com/neovim/nvim-lspconfig/blob/master/lua/lspconfig/server_configurations/phpactor.lua

foobar1643 commented 10 months ago

I don't see how this affects root_dir or LSP at all? The issue is in rpc.handle_input_callback() here: https://github.com/gbprod/phpactor.nvim/blob/main/lua/phpactor/rpc.lua#L122

It creates a UI prompt with default value of that prompt being taken from utils.path() function, which is just getting the name of the current buffer using neovim API: https://github.com/gbprod/phpactor.nvim/blob/main/lua/phpactor/utils.lua#L20

In my case, while in neo-tree, buffer name always ends with neo-tree filesystem [1000] which I have to delete from that prompt every time I want to create a class.

I'm not doing anything with the path to project's root directory used by LSP, this is more like a UI change. Perhaps naming for utils.path() is a little ambiguous?

gbprod commented 10 months ago

Oh! sorry, I've missunderstood the issue. I have to dive into it to fully understand the problem.

gbprod commented 10 months ago

ok, for the moment I think using vim.fn.expand("%:p:h") instead of vim.api.nvim_buf_get_name(bufnr) should fix your issue. I still investigating because I would like to be able to create new classes from neotree

foobar1643 commented 10 months ago

Thanks, I'll try that later. You can reproduce this by opening neo-tree like this: :Neotree position=current and doing :lua require('phpactor').rpc('new_class')

I've also noticed that utils.path() is used by various RPC handlers like new_class for example: https://github.com/gbprod/phpactor.nvim/blob/main/lua/phpactor/handler/new_class.lua#L6

But its value doesn't seem to have any effect on the result produced by these calls. During my testing I found that no matter what I put in current_path, phpactor still works with files/classes that were in the vim UI prompt.

foobar1643 commented 10 months ago

So I've tried using your suggestion in the path function and it somewhat works by not including incorrect buffer name in the prompt anymore, but now the problem is that the prompt only points to one directory in the project - the root directory.

If I have a directory structure like this:

phpactor-test/
├── src
│   ├── SubdirectoryOne
│   │   └── TestClassTwo.php
│   └── TestClassOne.php
└── vendor

And my cursor in neo-tree is at SubdirectoryOne I expect that by calling new_class RPC (or any other RPC that require the same parameters) the path would be phpactor-test/src/SubdirectoryOne/. Instead the prompt will always contain phpactor-test/ and the rest of the path (src/ and SubdirectoryOne/) I'll have to type in which isn't very convenient.

In my neovim configuration I've implemented such behavior by using the API neo-tree provides, but its very specific and I would imagine that other plugins which provide file trees do things differently, so my original idea was to let users of the plugin define their own path resolution functions.

This is what I'm doing to get the correct path based on currently selected directory, but as I've said this is neo-tree specific which is why it wasn't included in this PR:

local phpactor = require("phpactor")
local neoTreeManager = require("neo-tree.sources.manager")

phpactor.setup(
  {
    utils = {
      pathFn = function()
        local winid = vim.api.nvim_get_current_win()
        local state = neoTreeManager.get_state("filesystem", nil, winid)
        local node = state.tree:get_node()

        if node.type == "file" then
          return node.path:match("(.*[/\\])")
        end

        return node.path
      end        
    },
gbprod commented 10 months ago

Thanks for that, I'm working to make something that will works on most cases and I'm interested by your feedback : Can you try with this branch https://github.com/gbprod/phpactor.nvim/pull/26

foobar1643 commented 10 months ago

Since you're implementing a fix for this I'm closing this PR, lets move further discussion to #26