feat: Interactive uc command to select preferred cluster connection #170
Conversation
acaa6c2 to
f427cab
Compare
psviderski
left a comment
There was a problem hiding this comment.
Nice one! 👍
Please see the comment about simplifying the selection logic and removing unnecessary code for comparing connections.
BTW, did you consider implementing this as part of uc ctx as described in #125?
Any reason you decided to make it a separate command?
cmd/uncloud/context/connection.go
Outdated
| var selectedConnection *config.MachineConnection | ||
| form := huh.NewForm( | ||
| huh.NewGroup( | ||
| huh.NewSelect[*config.MachineConnection](). |
There was a problem hiding this comment.
I think this could be significantly simplified by using an int type (index) instead of MachineConnection for the selected item. In this case, we don't need to compare connections and maintain the Equal method.
You may also want to move the reordering logic to a method of the Context struct, something like currentCtx.SetDefaultConnection(index). It's up to you.
There was a problem hiding this comment.
Ah, yeah, good idea! The non-comparable MachineConnection due to the key's byte slice sent both Gemini and I down a rabbit hole on that one... 😭
I think I just misunderstood what you meant by "extend the uc ctx command" then. I figured you meant something like what I did here So are you thinking |
a890a45 to
9b3389a
Compare
|
I somehow missed the last bit from your comment #125 about extending
I just looked into doing that, and while I see we could have a second group/page with the connections selection after context, I don't see anything in Did you have any idea in mind on how you would implement that UI? Another option would be to make |
9b3389a to
8b47829
Compare
Not really, I just fantasised the UX I'd likely want to have but unsure what's the best way to implement it. Looks like it will require wrapping the form in a Bubble Tea program and calling its View/Update methods instead of Run.
It's not a terrible UX but I switch clusters often and in my 99.9% use cases I don't care about the connection so that will annoy me personally. I like having a separate command for selecting only the connection that this PR introduces, so let's have it regardless of trying to make the |
Implements #125 as
uc ctx connection