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

Rework find_root_dir #417

Closed
ckipp01 opened this issue Jun 22, 2022 · 2 comments
Closed

Rework find_root_dir #417

ckipp01 opened this issue Jun 22, 2022 · 2 comments
Labels
enhancement New feature or request feedback wanted

Comments

@ckipp01
Copy link
Member

ckipp01 commented Jun 22, 2022

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

--- NOTE: This only searches 2 levels deep to find nested build files.
--- Given a situation like the below one where you have a root build.sbt
--- and one in your module a, you want to ensure the root is correctly set as
--- the root one, not the a one. This checks the parent dir to ensure this.
--- build.sbt <-- this is the root
--- a/
--- - build.sbt <- this is not
--- - src/main/scala/Main.scala
local find_root_dir = function(patterns, startpath)
local path = Path:new(startpath)
-- TODO if we don't find it do we really want to search / probably not... add a check for this
for _, parent in ipairs(path:parents()) do
local pattern = has_pattern(patterns, parent)
if pattern then
local grandparent = Path:new(parent):parent()
-- If the pattern is found, we don't check for all patterns anymore,
-- instead only the one that was found. This will ensure that we don't
-- find a buid.sc is src/build.sc and also a .git in ./ causing it to
-- default to that instead of src for the root.
if has_pattern({ pattern }, grandparent) then
return grandparent.filename
else
return parent
end
end
end
end

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 #416 (reply in thread)

@ckipp01 ckipp01 added enhancement New feature or request feedback wanted labels Jun 22, 2022
@ckipp01 ckipp01 changed the title Better find_root_dir handling in deeply nested project structures Rework find_root_dir Dec 15, 2022
@ckipp01
Copy link
Member Author

ckipp01 commented Dec 15, 2022

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:

or { "build.sbt", "build.sc", "build.gradle", "pom.xml", ".scala-build", "bleep.yaml", ".git" }

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
Copy link
Member Author

ckipp01 commented Aug 17, 2023

Closed via #585

@ckipp01 ckipp01 closed this as completed Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feedback wanted
Projects
None yet
Development

No branches or pull requests

1 participant