-
Notifications
You must be signed in to change notification settings - Fork 16
feat: enhance chat interface #144
base: main
Are you sure you want to change the base?
Conversation
Demo Screen.Recording.2025-08-26.at.12.03.56.mov |
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.
Looks nice!
There was an initiative to have the same TUI as Gordon, has anything been decided on that? CC @krissetto
- Use bubbletea to implement a readline-like interface - Support arrow keys as well as the common readline keybindings for search - Implement /copy command for copying the latest response - Maintain backward compatibility with triple-quote formatting - When piping stdin into the run command, read the stdin and use it as the prompt Signed-off-by: Alberto Garcia Hierro <[email protected]>
I was having a go at implementing this yesterday but this is better! "Home" and "End" keys are worth implementing/testing also to navigate to the start and end of the string. |
One not so great thing about clipboard, is it's dependant on xlib for linux:
xlib is kinda dead in favour of Wayland-based solutions. |
Up and down should probably cycle through command history, could be in a follow on PR... In other similar tools like RamaLama and Ollama Ctrl-c stops the LLM from responding but doesn't quit the application, this is useful in case the LLM keeps talking. But Ctrl-d does quit. |
I wonder is the /copy functionality worth clipboard and all its dependencies (could be just a UI feature), goes above and beyond what a CLI interface normally does, one can highlight, copy and paste. |
commands/run.go
Outdated
|
||
text := textinput.New() | ||
text.Placeholder = placeholder | ||
text.Prompt = ">>> " |
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.
Kinda liked the single > prior to this change, two extra characters in a terminal for real text
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.
Makes sense, thanks! I've changed it back to a single >
.
Signed-off-by: Alberto Garcia Hierro <[email protected]>
That's already implemented in this PR.
We could do that in a followup. |
I tried and with this branch and it didn't seem to work, let me try again 😊
|
Home and End worked, I tried |
The up and down arrows work on my machine with the latest commit, but some oddities noticed:
|
There's an odd bug now, type "Hello", press up twice "1Hello" appears:
|
I cannot reproduce this. Could you please delete |
That worked, maybe 1Hello was in my history... The other difference I see between this and bash/ollama/etc. is when you cycle back down again, normally the last down entry in blank. Here it's not. |
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.
I like this functionality overall, but I'd like to discuss it with @mia-docker first since this is a very UI/UX-heavy change. I'd also like to discuss how we plan to implement chat history in Docker Desktop with @qodesmith (who will be back next week) to make sure that we're aligned on storing those that data. @fiam are you okay with one of us following up on this PR next week once we discuss further? I think others on the team could contribute to it as well in case you're time-constrained at that point.
"github.com/docker/model-cli/desktop" | ||
"github.com/docker/model-cli/pkg/history" | ||
"github.com/spf13/cobra" | ||
"golang.design/x/clipboard" |
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.
Instead of using golang.design/x/clipboard
that relies on cgo, can we just use the github.com/atotto/clipboard
dependency that we've already brought in via bubbletea
? The API looks like it would provide the same functionality we need, and it already operates via shelling our rather than using cgo.
// New creates a new History instance and loads all previous history, if it exists. | ||
func New(cli *command.DockerCli) (*History, error) { | ||
dirname := filepath.Dir(cli.ConfigFile().Filename) | ||
p := filepath.Join(dirname, "model-cli", "history.txt") |
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.
Perhaps we should discuss where we want to store this, because we also have chat history as a near-term roadmap item for the GUI, and I think it would be nice if we could share chat output between them. I get that this is more of a readline()
-style history, but I think we should discuss with @mia-docker and @qodesmith about how we want the two to interact.
Convering to draft while we discuss this approach, to prevent an accidental merge. |
Signed-off-by: Alberto Garcia Hierro [email protected]