search

scalameta / nvim-metals

A Metals plugin for Neovim
https://scalameta.org/metals/
Apache License 2.0
463 stars 74 forks source link

Rework `find_root_dir` #417

Closed ckipp01 closed 1 year ago

ckipp01 commented 2 years ago

Alright so I just took a look at neo4j, this behavior actually make sense, and matches what I mentioned above. The logic in nvim-metals is to go two layers deep. You can see this in https://github.com/scalameta/nvim-metals/blob/58ecfb61e4617139d3954138ccccc4c0befa89e0/lua/metals/rootdir.lua#L14-L40

So it finds the first pom.xml in community/community-it/cypher-it/pom.xml and then the second inside of community/community-it/pom.xml and then goes no further, and marks that as root. In more simple builds, and in most sbt projects (which most users use), this is no issue, and works like 95% of the time. However you're not the first one to hit on this, in reality, you're the 3rd one to create an issue/question about it, so I think it's time to change this logic.

I see two different ways forward.

  1. We traverse down until we don't find anymore build files. So in this example the room pom.xml would be found and then the parent directory wouldn't have one, so then the root would be correctly marked. The only danger of this is that technically we'd be looking at the parent of the actual workspace at times, and I really want to avoid ever going too deep and marking something not even in your workspace as the root. Will this ever happen? No idea, but who knows.
  2. Secondly, and someone else suggested this would be to add in a custom depth setting. Right now it searched for 2, but a user could pass in 4 for this situation, and then it would continue to look for pom.xml files 4 layers deep. This could also work, but would require the user to see how deep they are nested and mark it.

I'm not 100% sure which option would be best so I might think on this a bit. I'll create an issue out of this conversation. In the meantime, something you could do is override the find_root_dir with your own function that finds the root or just sets it like this:

metals_config.find_root_dir = my_find_root_dir

Then my_find_root_dir is just a function that takes in patterns and a startpath. For example others have done this using an env var.

Let me know if you have any questions on this for now as a workaround, and I'll hopefully get a proper fix for this soon.

Originally posted by @ckipp01 in https://github.com/scalameta/nvim-metals/discussions/416#discussioncomment-3005345

ckipp01 commented 1 year ago

Alright so there is one other issue with the current logic that makes me want to re-think a bit more how we do this. CC @keynmol since it was thanks to you that I realized this.

So currently for all or our root patterns that can be found in:

https://github.com/scalameta/nvim-metals/blob/d1c01907256dae7c9d55ba1fcfb8cf6b4f583325/lua/metals/config.lua#L319

We treat them all the same and with the logic that exists in https://github.com/scalameta/nvim-metals/blob/main/lua/metals/rootdir.lua we always ensure that there isn't nested build files. If we find them, then we continue down to the second layer and grab that. This is more or less desired in sbt, maven, or gradle builds that have this pattern, but not others like .scala-build, .bsp, or .git. A more robust solution would be to treat the patterns that have nested structures differently and continue the search for nested build files only for those, and not for the other ones.

ckipp01 commented 1 year ago

Closed via https://github.com/scalameta/nvim-metals/pull/585