Adding support for .slns, among other enhancements#2
Adding support for .slns, among other enhancements#2haondt wants to merge 9 commits intod7omdev:mainfrom
.slns, among other enhancements#2Conversation
|
Hey @haondt, this is really cool, thanks for putting this together. The async live-search alone is a huge UX improvement over the old input prompt approach, Glad you decided to share it. I went through everything carefully and found a few bugs worth fixing before I merge: Bug 1 — version aggregation picks the lowest, not the highest (dotnet.lua lines 96, 173, 211, 261) After table.sort(versions, utils.version_lt) the list is ascending, so versions[1] is the lowest version. The annotation says it should be the highest — fix is just Bug 2 — crash in the remove failure path (pickers/csproj.lua:69) notify.show_error_float("Failed: " .. package .. " " .. sel.value, ...) package isn't in scope here, it resolves to Lua's own built-in package table and blows up with a concatenation error. Should be sel.value.id. Bug 3 — other_cnt is off (pickers/nuget.lua:246) local other_cnt = #projs -- already the full count Say a package is in 3 projects and you're targeting 1 of them — the badge ends up showing (5) instead of (2). Starting other_cnt at 0 fixes it. Bug 4 — build_project_map always calls bare dotnet (dotnet.lua:516) vim.fn.systemlist("dotnet sln " .. vim.fn.shellescape(...) .. " list") Whatever the user set for dotnet_bin gets ignored here. Also worth noting that this function has a hard dependency on fd with no fallback — if it's not installed the Bug 5 — is_sln never actually set (pickers/projects.lua:54) The display function checks et.is_sln to pick the highlight, but nothing ever sets it on the entry. Adding is_sln = filetype == "sln" to the entry table is all it needs. Smaller things I noticed:
Bugs 1–5 are the ones I'd want fixed before merging, the rest can be follow-ups. Let me know if anything's unclear |
This is a pretty big change, feel free to close the PR if you're not interested. I have no intention of breaking up this change (I can make additional changes/fixes if needed), but felt I should at least offer to contribute back. It started with a smaller change and kind of spiralled.
Changes:
NuGet.Configwhen searching packages.csprojand.slnfiles.I believe it also addresses some, but not all of the concerns in #1. It can update and consolidate multiple csprojs under the same solution, but some more work would need to be done to install a package on discrete projects.