find_node_modules_ancestor does not always return parent

[mikehaertl]

Description

The documentation says:

• util.find_node_modules_ancestor: a function that locates the first parent directory containing a node_modules directory.

This is not quite correct: It does not return the parent directory, if there's already a node_modules dir in the passed directory.

So either the documentation is wrong or the behaviour should be fixed.

Neovim version

NVIM v0.7.0 Build type: Debug LuaJIT 2.1.0-beta3 Compilation: /usr/bin/cc -DNVIM_TS_HAS_SET_MATCH_LIMIT -DNVIM_TS_HAS_SET_ALLOCATOR -g -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wshadow -Wconversion -Wdouble-promotion -Wmissing-noreturn -Wmissing-format-attribute -Wmissing-prototypes -Wimplicit-fallthrough -Wsuggest-attribute=pure -Wsuggest-attribute=const -Wsuggest-attribute=malloc -Wsuggest-attribute=cold -Wvla -fstack-protector-strong -fno-common -fdiagnostics-color=always -DINCLUDE_GENERATED_DECLARATIONS -D_GNU_SOURCE -DNVIM_MSGPACK_HAS_FLOAT32 -DNVIM_UNIBI_HAS_VAR_FROM -DMIN_LOG_LEVEL=1 -I/home/repos/github/neovim/build/config -I/home/repos/github/neovim/src -I/home/repos/github/neovim/.deps/usr/include -I/usr/include -I/home/repos/github/neovim/build/src/nvim/auto -I/home/repos/github/neovim/build/include Übersetzt von mike@Nilar

Features: +acl +iconv +tui See ":help feature-compile"

      System-vimrc-Datei: "$VIM/sysinit.vim"
 Voreinstellung für $VIM: "/usr/local/share/nvim"

Nvim-lspconfig version

0f06f7b

Operating system and version

Linux Mint 20.3 Una

Affected language servers

volar

Steps to reproduce

mkdir -p /home/mike/test/a/node_modules
mkdir /home/mike/test/node_modules

Then in neovim:

:lua print( require'lspconfig'.util.find_node_modules_ancestor('/home/mike/test/a') )

Actual behavior

It echoes /home/mike/test/a

Expected behavior

It should echo the first parent dir with a node_modules, i.e. /home/mike/test/.

Minimal config

Not relevant

LSP log

not relevant

[mikehaertl]

It's probably a definition thing but I would not consider that a directory is part of its ancestors. Problematic code is here:

https://github.com/neovim/nvim-lspconfig/blob/0f06f7ba286efd4ae187abd3391f1574580ff929/lua/lspconfig/util.lua#L312-L316

In my understanding lines 314-316 should be removed. Alternatively the method's description should be changed to make it clear that it's ancestor or the directory itself.

[lithammer]

I suspect the confusion comes from the fact that the function is meant to take the path to a file, not a directory.

[mikehaertl]

Yeah, maybe, but even then: if someone talks about "the parent directory of a file" I would probably still expect the parent of the directory that the file is located in. But maybe that's just me.

In any case it could be fixed so that it also works nice with directories. I'm just not sure if this would affect other parts.

[johnsoncodehk]

For your information, you can also find typescript path with node process.

https://github.com/sublimelsp/LSP-volar/issues/19#issuecomment-914388298 https://github.com/sublimelsp/LSP-volar/blob/master/server/resolve_module.js

[mikehaertl]

@johnsoncodehk I think your comment was meant to go here: https://github.com/neovim/nvim-lspconfig/pull/1909 ?

[glepnir]

Does this problem still exists ?

[mikehaertl]

@glepnir Neither code nor documentation was updated.

But as discussed above maybe I'm alone with my opinion, that "ancestor of X" does not include X, even if X is a directory.

[glepnir]

maybe we can do something before search like

function M.find_node_modules_ancestor(startpath)
 if path == startpth then
  change the startpath value
 end
  return M.search_ancestors(startpath, function(path)
    if M.path.is_dir(M.path.join(path, 'node_modules')) then
      return path
    end
  end)
end

[mikehaertl]

Hmm, what does path refer to in the second line of your example?

I'd rather think something like this should work:

if M.path.is_dir(startpath) then
   strip rightmost directory name from startpath
end

[glepnir]

now you open test.ts then open test_b.ts the server .it will reuse typescript-language-server and the root is ProjectA . note now the most correct name of root is workspace. I will change the lspinfo next . now when you open a new project lspconfig will check does it include a already opened project. if exist this project belong to the exist project workspace. if not it's a new workspace.

Project A 
         - node_modules
         - test.ts
         - Project B
         - node_modules
         - test_b.ts