rnix-lsp
rnix-lsp copied to clipboard
Cleanup
The biggest change here is the cleansing of all outstanding clippy lint warnings. I had to add an exception for clippy::cast_possible_truncation
as the lsp_types::Position
struct requires a u32, but we are currently building it from a usize.
I also ran a cargo fmt
on the codebase, removed the implicit nix-direnv dependency on the envrc, added some rust development tools to the devshell and pulled rust from unstable to get a slightly newer version.
One of the lints corrected a scenario where a value's Drop
destructor was never called, so there may be a miniscule chance that this was the root cause of #33.
I like these changes. Thank you @nrdxp! I also agree with @Ma27's feedback :-)
Just tried it out. Seems like a way to trigger the memleak is to open a let
binding but not close it, then add an inherit
. Instant trigger.
Wait.... I think... I can reproduce the memleak, realiably :tada: (and I kinda feel dumb now :D)
I halted both affected rnix-lsp
processes with gdb
on my laptop for now since I'm actually busy with studying right now, but I'll try to take a look later, thanks! :tada:
Also, I'm willing to fix the merge conflict while merging this PR, rather than making @nrdxp fix it (unless they want to)
@nrdxp may I ask if you intend to finish this? Otherwise I'd take care of it when i'll get to it :)
sorry, I completely forgot about this. I wouldn't mind rebasing it and finishing it off, but if you have the time and the motivation, feel free to take over. If not, I'll schedule it for this weekend. That said, it might be interesting to also bump the rust edition now that 2021 is stable, although it's a fairly minor change all things considered so it may not make much of a difference.
If not, I'll schedule it for this weekend
If you still have the capacity, feel free to finish :) I wouldn't have managed to take care of it last week (and most likely also not this weekend).
. That said, it might be interesting to also bump the rust edition now that 2021 is stable, although it's a fairly minor change all things considered so it may not make much of a difference.
Oh, good point, thanks for the reminder!
Feel free to try it out :)
just fyi, I started on refactoring this on the weekend. Unfortunately didn't finish since I was rather busy with family stuff, but I'm hoping I should be able to get it done sometime in the next week.