feat: ✨ add minimal tui for metadata exploration#331
Conversation
lwjohnst86
left a comment
There was a problem hiding this comment.
Neat idea, but this PR is way too big and the code doesn't match our style at all. Did you use AI for this? 🤔
There was a problem hiding this comment.
This code reminds me of AI generated code... is it? If it isn't, this needs to be heavily refactored to follow our style and clean it up. E.g. there are several nested for loops, a lot of conditionals, a lot of use of list comprehensions, capitalisations when we only use capitalisations for constants, and too deeply nested code.
There was a problem hiding this comment.
Yeah, this is codex via opencode, same as I mentioned I've been using as a coding assistant for some time. Just to be clear, it's not a one pass "do this and done", there is a lot of iteration but I'll admit that I got a bit too excited about having a prototype up and working that I forgot to adjust the coding style to be functional etc =p That's my bad and I will address that here after we finish the brainstorming about the design in #330 so that I know if we keep --mode approach here or go for a separate subcommand (and potentially other design decisions).
But aside from coding style, does the Textual approach with components, CSS, etc make sense and is that a dependency you can see us take on in flower? I don't have experience with it from previously so I don't know of any gotchas, just that it's built by the same people that made rich and generally popular for making TUIs (in python in particular, I could not find anything else of similar capacity).
I also thought it would be helpful to have this up so that anyone in the team can take it for a spin while we do the brainstorming to experience how things work and not just watch the video I posted.
There was a problem hiding this comment.
@lwjohnst86 Just a ping in case this comment got lost, no rush though.
There was a problem hiding this comment.
I think we decide based on the discussion in #330 and maybe make a decision post about which tool to use for TUIs in Python? Unless we refactor this to Rust, which means we'd need to go with a different tooling.
There was a problem hiding this comment.
Ok, I'll leave this until we arrive at something in #330 and maybe also take a decision on whether we want to use Rust here or not (maybe after we learn more about it for propagate).
Description
See motivation in #330. There is also a more fully fledged demo there. Here is a screenshot of what the basic implementation in this PR would look like
Close #330, close #327, maybe also close #259?
Needs a thorough review.
Checklist
just run-all