-
Notifications
You must be signed in to change notification settings - Fork 648
Mitchdenny/run-apphost-logs #9952
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
Conversation
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.
Pull Request Overview
This PR updates resource strings used by the Aspire CLI application and introduces a new dashboard rendering for app host logs. Key changes include:
- Refactoring resource strings to shorten text and relocate description notes.
- Adding a new trans-unit for displaying a prompt to view app host logs in various locales.
- Implementing a new dashboard rendering view and updating keyboard handling in RunCommand.
Reviewed Changes
Copilot reviewed 19 out of 22 changed files in this pull request and generated 13 comments.
File | Description |
---|---|
src/Aspire.Cli/Resources/xlf/RunCommandStrings.*.xlf | Updated resource strings for resource health and added a new prompt for viewing app host logs. |
src/Aspire.Cli/Resources/RunCommandStrings.resx | Updated resource strings similarly for the default resources file. |
src/Aspire.Cli/Rendering/Dashboard/* | Added new dashboard renderables and state management for app host logs. |
src/Aspire.Cli/Commands/RunCommand.cs | Enhanced RunCommand with dashboard integration and keyboard input handling. |
Files not reviewed (1)
- src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs: Language not supported
@@ -151,3 +151,4 @@ playground/**/publish/ | |||
|
|||
# Verify | |||
*.received.* | |||
**/.aspire/settings.json |
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.
Why?
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.
Just figured it would be helpful to folks since everyone is often using a different playground project.
|
I'll look at clearing the screens when we flick. Regarding clearing the table when we CTRL-C -- we kind of have to do it that way to make the logs thing work. We are using an alternate buffer / screen to make this all work. So, when you step back from the alternate buffer / screen you get the original content back (it's not that the table is cleared, it's that it was never on that original buffer to begin with). |
Scrolling also seems pretty broken with this change. |
Fixes: #9622
This change changes the way that we render the terminal dashboard for
aspire run
. Here is a summary of the changes:_ansiConsole.Live(...)
call and changed the callback code forLive(...)
to effectively be a render loop.DashboardState
type which is passed to a set of background threads that do things like process keyboard input, stream resource states, and get dashboard URLs.This isn't a replacement for
aspire run --debug
but its something that you can use to get a quick glance at apphost logs. This is also not a fully blown TUI since that is a lot larger effort (based on some spiking that I did):aspire-run-tui-light.mp4