-
Notifications
You must be signed in to change notification settings - Fork 15
Progress bar with more info #143
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
base: main
Are you sure you want to change the base?
Conversation
Looks like this:
|
@xenoscopic @p1-0tr PTAL... Especially as regards user-facing aesthetics... Want to avoid wasted effort if the appearance isn't preferred... |
Needs a bit more work... |
ee17d51
to
666c1d4
Compare
This is complete and ready for review/merge... PTAL need somebody to press the build button again @xenoscopic @p1-0tr |
3a0fab7
to
524737d
Compare
More info than previous progress bar Signed-off-by: Eric Curtin <[email protected]>
524737d
to
ab36e74
Compare
return fmt.Sprintf("%s%s| %s", prefix, bar, suffix) | ||
} | ||
|
||
func getTerminalWidthUnix() (int, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to go in a separate file with build directives (https://pkg.go.dev/cmd/go#hdr-Build_constraints) to make sure the Unix specific code is built only on Unix. You can use https://github.com/docker/model-runner/tree/main/pkg/gpuinfo as an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy for the change overall. We'll likely be doing concurrent downloads for sharded GGUF layers in the next release, so detailed and dynamic progress bars would be great (I'm not sure how much of an extension it would be for this implementation to support multiple bars).
Agree with @p1-0tr that the main thing would just be making sure it works on Windows (or at least builds with a simple fallback implementation). On the Windows side, we do get a lot of users on MinTTY-based terminals, in which case maybe it's worth using an existing library to implement this (e.g. the progress bar component of https://github.com/charmbracelet/bubbles (which uses https://github.com/charmbracelet/bubbletea)). @fiam might know best.
More info than previous progress bar