Skip to content
This repository has been archived by the owner on Jan 7, 2025. It is now read-only.

Add Textual #502

Merged
merged 24 commits into from
Feb 13, 2024
Merged

Add Textual #502

merged 24 commits into from
Feb 13, 2024

Conversation

PCSwingle
Copy link
Member

@PCSwingle PCSwingle commented Jan 22, 2024

Adds textual

Pull Request Checklist

  • Documentation has been updated, or this change doesn't require that

@PCSwingle PCSwingle changed the title first textual commit Add Textual Jan 26, 2024
@PCSwingle PCSwingle marked this pull request as ready for review February 5, 2024 23:27
@waydegilliam
Copy link
Contributor

image

I think the default background/theme should match whatever the terminal has its theme set to. Pretty sure there's a way to do this easily with Textual.

@waydegilliam
Copy link
Contributor

image Should we be rendering the code blocks with syntax highlighting? Also Textual can render Markdown (for the model response that isn't a code edit) which could look nice.

I'm using the default Macos terminal here btw

@waydegilliam
Copy link
Contributor

image Seeing weird line breaks on the sides of the input box, guessing it's due to the Macos terminal not being TrueColor and/or not supporting ligatures

@waydegilliam
Copy link
Contributor

waydegilliam commented Feb 6, 2024

Would be nice to get some kind of "loading" spinner or progress bar after I submit a prompt. Currently there's no feedback after the user response to an input request for a while which makes me feel like it's hanging on something. This is mainly for when we have a really large context that takes a while to process.

@PCSwingle
Copy link
Member Author

PCSwingle commented Feb 6, 2024

Seeing weird line breaks on the sides of the input box, guessing it's due to the Macos terminal not being TrueColor and/or not supporting ligatures

Oh yeah probably since I don't have those. Any idea on how to fix those?

Should we be rendering the code blocks with syntax highlighting? Also Textual can render Markdown (for the model response that isn't a code edit) which could look nice.

We never render code blocks that aren't code edits; it would definitely be a nice feature though. I'll look into it; the only problem I can think of right now is that we're streaming the response, so it might behave weirdly (like it starts out streaming and you see the ```tsx up until it finishes the block, and then it finally renders everything). I'll test it out but I think I agree that it'll be better than what it currently is.

@PCSwingle
Copy link
Member Author

I think the default background/theme should match whatever the terminal has its theme set to. Pretty sure there's a way to do this easily with Textual.

I think you're right and I'll look into it; my textual is always full screen though; why is yours not? Is it because you're using tmux? Either way I'll try and figure this out.

@PCSwingle
Copy link
Member Author

PCSwingle commented Feb 6, 2024

@waydegg https://textual.textualize.io/FAQ/#why-doesnt-textual-look-good-on-macos
Maybe we should add this to the docs? Can you let me know how much it fixes?

@waydegilliam
Copy link
Contributor

https://textual.textualize.io/FAQ/#why-doesnt-textual-look-good-on-macos
Maybe we should add this to the docs? Can you let me know how much it fixes?

Much better! Def worth adding in the docs
image

@waydegilliam
Copy link
Contributor

I think you're right and I'll look into it; my textual is always full screen though; why is yours not? Is it because you're using tmux? Either way I'll try and figure this out.

Yeah full screen looks great, and yup using Tmux so it looks a bit off. It looks like there might be a way to do this with Rich: https://rich.readthedocs.io/en/stable/syntax.html?highlight=terminal%20theme#background-color

The creator of Textual just opened an issue a couple days ago talking about this actually: Textualize/textual#4119

@waydegilliam
Copy link
Contributor

Would be nice to get some kind of "loading" spinner or progress bar after I submit a prompt

A couple users mentioned wanting this in the Discord server too. Worth adding later imo

@PCSwingle
Copy link
Member Author

PCSwingle commented Feb 7, 2024

Just finished looking into all of your comments;

  1. I tried displaying Markdown; it came with a lot of problems though. For example, a lot of the time the model wouldn't close off its markdown ``` tags and all the text after that would be marked down. Also, markdown doesn't support color. Overall I might be able to fix some of these problems, but it seems like too much work for very little gain to me (how often does the model output markdown formatted text anyways? Not too often).

  2. I don't think there's a way to check the background of the terminal, but I did hook up the config.theme to Textual, so setting your theme to dark or light mode will set the background. I think this is fine, as most people will be opening it up in full screen, so it will look normal, and they can set their config to dark mode if they use a dark mode terminal (if they open it up in a non full screen mode somehow then hopefully they can figure it out haha).

  3. Added MacOS bug fix to docs!

Copy link
Member

@jakethekoenig jakethekoenig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! I think the following would be nice but not necessary to ship:

  • x buttons in context pane to remove files.
  • A microphone emoji in the entry box to trigger /talk
  • The ability to change theme midsession with /config. The UX here is strange because presumably users will almost never switch back and forth. I wonder if there should be a sticky /config that saves it to the config or if some settings should be sticky and others not.
  • This pr breaks reverse search. It'd be nice to get that back.

I'll keep using it this morning and let you know if I notice anything.

mentat/command/commands/context.py Outdated Show resolved Hide resolved
@biobootloader
Copy link
Member

when you exit should it display something in the terminal? like total conversation cost?

image

@biobootloader
Copy link
Member

unique widget ID error:

image

@biobootloader
Copy link
Member

I can't select text (like copying the above error). Looks like this requires holding option / shift, but those don't work for me. Does option work for anyone else on a mac? maybe my window manager is messing it up - option moves the window around

image

@granawkins
Copy link
Member

I can't select text (like copying the above error). Looks like this requires holding option / shift, but those don't work for me. Does option work for anyone else on a mac? maybe my window manager is messing it up - option moves the window around

Also not working for me on mac - tried lots of combinations, can't select/copy.

Copy link
Member

@granawkins granawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few small comments. My wish list would be:

  1. Loading Bar issue
  2. Copiable text
  3. Dark theme (wasn't working out-of-the-box for me but maybe I'm missing sth?)

mentat/config.py Show resolved Hide resolved
mentat/terminal/client.py Show resolved Hide resolved
position: int = 0


class PatchedDropdown(Dropdown):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small feature request - take it or leave it - I always forget that 'Tab' selects sth from the dropdown. Maybe the bottom line of the widget could say "(Press 'tab' to select)" or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't change the text size (since we're working in a terminal), so I don't think this would end up looking very good. Maybe a different key would be better than tab?

Copy link
Member

@granawkins granawkins Feb 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could it be Tab | Enter?

@PCSwingle
Copy link
Member Author

Looks great to me! I think the following would be nice but not necessary to ship:

  • x buttons in context pane to remove files.

This would be pretty difficult to do; I don't even know if it's possible. Right now I use the Tree textual widget and I looked at the docs and couldn't find anything that would do this. I'll make an issue though and I can look into it later, because I really do like this idea.

  • A microphone emoji in the entry box to trigger /talk

Would probably be easier, but I also think it would be good in another PR since I'm not entirely sure yet how I would do it. I'll put this in the same issue as the x buttons.

  • The ability to change theme midsession with /config. The UX here is strange because presumably users will almost never switch back and forth. I wonder if there should be a sticky /config that saves it to the config or if some settings should be sticky and others not.

This would be pretty nice; the reason I disabled mid session change is because then we need something watching the config to see when it changes and alert the terminal app (not because it's actually difficult to switch the theme midsession). A sticky config would be nice but then the question is where we put the stored config; we have like 3 different config files. Not sure how to address this, but I'll make another issue for it.

  • This pr breaks reverse search. It'd be nice to get that back.

I don't think this is the biggest deal and it would be very difficult to add (would have to program in our own reverse search).

I can't select text (like copying the above error). Looks like this requires holding option / shift, but those don't work for me. Does option work for anyone else on a mac? maybe my window manager is messing it up - option moves the window around

I can't really test this without a mac; if somebody else could try to figure this out and fix it that would be great. Thanks!

@PCSwingle
Copy link
Member Author

  1. Dark theme (wasn't working out-of-the-box for me but maybe I'm missing sth?)

What problems were you having with it? Did you change a config file or run with --theme dark?

@PCSwingle PCSwingle mentioned this pull request Feb 12, 2024
@granawkins
Copy link
Member

granawkins commented Feb 13, 2024

  1. Dark theme (wasn't working out-of-the-box for me but maybe I'm missing sth?)

What problems were you having with it? Did you change a config file or run with --theme dark?

Ah ya I used --theme dark and it works. I saw Wayde said it'd be nice if it picked up your theme automatically, so I was echoing that. But also, low priority.

@biobootloader
Copy link
Member

Ok I figured out why holding Option key wasn't working for me - my window manager Yabai moves windows when you click on them while holding option. I turned Yabai off and selecting text now works - but selecting multiple lines gets you context from the right side, which isn't nice:

image

I'm going to go ahead and merge this though!

@biobootloader biobootloader merged commit b26f901 into main Feb 13, 2024
16 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants