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

rust-analyzer: Fix is_library #2995

Merged

Conversation

jeremija
Copy link
Contributor

@jeremija jeremija commented Feb 2, 2024

I'm not that well versed in Lua, but the previous check

if fname:sub(1, #item) == item

didn't really work as expected and I was still experiencing high CPU
usage when using the go to definition functionality. After adding some
print statements for debugging, I noticed that only a few chars were
removed from the base dir and thus the comparison was failing. Perhaps
it might work differently in other operating systems, but my Linux
machine it did not work correctly.

I replaced this check with util.path.is_descendant which now works
correctly, as well as added another path to check when git repositories
are used as cargo dependencies.

@jeremija jeremija requested a review from glepnir as a code owner February 2, 2024 11:18
@jeremija jeremija force-pushed the jeremija/fix-rust-analyzer-root-dir-check branch from e8b2087 to 4d315c5 Compare February 2, 2024 11:19
I'm not that well versed in Lua, but the previous check

   if fname:sub(1, #item) == item

didn't really work as expected and I was still experiencing high CPU
usage when using the `go to definition` functionality. After adding some
print statements for debugging, I noticed that only a few strings were
removed from the base dir and thus the comparison was failing. Perhaps
it might work differently in other operating systems, but my Linux
machine it did not work correctly.

I replaced this check with `util.path.is_descendant` which now works
correctly, as well as added another path to check when git repositories
are used as cargo dependencies.
@jeremija jeremija force-pushed the jeremija/fix-rust-analyzer-root-dir-check branch from 4d315c5 to 5efd0a6 Compare February 2, 2024 11:21
@glepnir
Copy link
Member

glepnir commented Feb 2, 2024

LGTM thanks :)

@glepnir glepnir merged commit 9a62799 into neovim:master Feb 2, 2024
9 checks passed
@jeremija
Copy link
Contributor Author

jeremija commented Feb 2, 2024

Cool, thanks for the blazing fast review and merge!

@jeremija
Copy link
Contributor Author

jeremija commented Feb 2, 2024

Actually, I got confused, the fname:sub(1, #item) == item did work as expected, but I was running into this issue only because my deps being in a git repo. So that part could've stayed, but I suppose util.path.is_descendant(item, fname) is still easier to grok 🙂

@glepnir
Copy link
Member

glepnir commented Feb 2, 2024

because my deps being in a git repo.

for some special cases, the directory cannot be judged by truncation maybe 🤔

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