Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Spec for Quick Fix #17005

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add Spec for Quick Fix #17005

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d451567
Add Spec for Quick Fix
carlos-zamora Apr 2, 2024
2447672
address feedback
carlos-zamora Apr 3, 2024
d95b594
a lil more feedback
carlos-zamora Apr 4, 2024
ee4c596
apply feedback from spec review
carlos-zamora Apr 5, 2024
37af517
add some future considerations (and other feedback)
carlos-zamora May 8, 2024
33cd4b6
spellcheck
carlos-zamora May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 58 additions & 0 deletions doc/specs/#16599 - Quick Fix.md
View file
Open in desktop
@@ -0,0 +1,58 @@
---
author: Carlos Zamora @carlos-zamora
created on: 2024-04-01
last updated: 2024-04-03
issue id: "#16599"
---
# Quick Fix
## Solution Design
Quick Fix will be a new UI surface that allows the user to interact with the terminal and leverage the context of a specific command being executed. The UI is discussed in the [UI/UX Design section](#ui/ux-design).

The UI and any logic populating the UI will entirely live in the TerminalControl project (similar to the context menu). The suggestions will be appended as top-level items, but a nested design can be revisited as more sources are added to populate the Quick Fix menu. Only one Quick Fix menu will be available at a time as it only appears on the latest prompt if an error occurred; this is in alignment with VS Code's design.
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

The actual suggestions will be populated from various sources such as the following:
- WinGet Command Not Found OSC Sequence:
- `OSC 9001; CmdNotFound; <missingCmd>` will be used by the attached CLI app to notify the terminal that a command `missingCmd` wasn't found. The TerminalControl layer will use WinGet's API to search for any packages that would remediate this error and construct a list of suggested packages to be installed.
`OSC 9001` can be expanded on to add more suggestions via VT sequences from the underlying CLI app; for more on this, read the [Future considerations section](#future-considerations).

Upon receiving an `OSC 9001; CmdNotFound`, the `missingCmd` parameter will be stored in the text buffer's `ROW`. Specifically, as of PR \#16937, `ROW` will contain an optional `ScrollbarData` struct to hold any metadata (i.e. scrollbar marks). We can stuff a `std::wstring missingCmd` into that metadata and populate it if `OSC 9001; CmdNotFound` was received.
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
`ControlCore` will be notified upon receiving an `OSC 9001; CmdNotFound`. At this point, we'll know that a Quick Fix menu must be shown at the next prompt, and we can use the WinGet API to convert the given `missingCmd` into a list of suggestions (in this case, of the form `winget install...`). Since we're in the TerminalControl layer, the suggestions will not need to be converted into SendInput actions, as we already have access to the `RawWriteString()` api. The menu will simply use `RawWriteString()` to inject the string into the prompt.

A corner case that VS Code handles is that if there is already text in the prompt area, it sends a `^C`, then injects the suggestion, then executes it. Terminal will have to operate the same way and, thus, must also be able to detect if there is already text in the prompt area.
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

An additional corner case is handling a buffer resize and reflow. Since the `missingCmd` will be stuffed as metadata in the text buffer's `ROW`, we should be able to generate the WinGet suggestions again fairly easily, if needed. Ideally, the generated suggestions can be cached and the quick fix menu is simply redrawn in the new reflowed position.

The WinGet API to convert the given `missingCmd` into a list of suggestions may be relatively slow as we have to query the WinGet database. Performing the search when the quick fix menu is open would not be ideal as the user may have to wait for WinGet to finish searching. Instead, the conversion should happen around the time that the prompt is introduced (whether from output or a reflow scenario).
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

## UI/UX Design
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
A Quick Fix icon will be displayed immediately left of the prompt. If the icon fits in the left-padding region (a `padding` value where the left side is 16 or greater), it will be drawn there. Otherwise, a thin accent-colored vertical bar will be displayed overlapping the text region; when the icon is hovered over, the icon will appear more clear by becoming more opaque and enlarging to overlap with the text region.
Copy link
Member

Choose a reason for hiding this comment

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

accent color is an interesting choice - why that over like, the foreground color? Or yellow from the scheme's color table?

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally, I feel like accent color is a good way to signify that the UI is personalized to the user from by the system (vs color scheme colors being a part of the standard theming). Accent colors just pop more, imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

@zadjii-msft here's the UI I have from my branch.

Normal version:
quickFix demo

Collapsed version:
quickFix demo (collapsed)

I did change it so that it's not just based on the "padding". It's a little more complicated than that, but basically we check if the button would "fit" in the gutter (with the icon being a similar size to the buffer font size).

Copy link
Member

Choose a reason for hiding this comment

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

oh my god it's so cool

Copy link
Contributor

Choose a reason for hiding this comment

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

@zadjii-msft here's the UI I have from my branch.

Looks slick! One question though - is the button supposed to show up on the prompt with the error in it? The GIF shows the button showing up on the next empty prompt line, which IMO is a little unintuitive

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup! Intentional to align with VS Code's design. I think it's so that if you had a lot of output, you don't have to scroll all the way back up to get to the quick fix menu.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense! Should the suggestion get input into that empty line then? Like in the image below. (Not sure if that's just a demo bug, just making sure I'm not missing it if it's in the spec!)

image

Copy link
Member

Choose a reason for hiding this comment

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

eh, it's probably because of the secret ^C we insert before the command to make sure it doesn't conflict with input the user has already typed

Copy link
Member Author

Choose a reason for hiding this comment

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

eh, it's probably because of the secret ^C we insert before the command to make sure it doesn't conflict with input the user has already typed

Yup, exactly that. We're always sending over the ^C for now, which is why we end up getting that blank line. But hopefully sometime in the future (perhaps with shell integration) we won't need to do that always.


When the icon (exposed as a button) is invoked, the Quick Fix menu (exposed as a flyout menu) will be displayed. This menu will include the following:
- WinGet Command Not Found:
- If a command wasn't found in the immediately previous output (between prompts), this entry will be enabled. It includes a nested menu of all the WinGet suggestions. If no suggestions were found or the previous output didn't have a missing command, this entry will be disabled.
- App Suggestions:
- The underlying CLI app could specifically recommend a command using a custom VT Sequence. For more information, read the [Future considerations section](#future-considerations).
These suggestions will be displayed as "Run: \<command\>", which aligns with the design used in VS Code's Quick Fix menu.

carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
## Accessibility
When a Quick Fix is available, an accessibility notification should be dispatched to alert the user that it is available. Aside from that, Quick Fix must be keyboard accessible via a default key binding. The UI will be a WinUI flyout menu, so it will be accessible by default once it is displayed and under focus.

## Future considerations
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
The Quick Fix menu can also be a home for future extensions to live. We can and should take inspiration from VS Code's implementation[^2][^3].
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

Other possible sources of suggestions in the quick fix menu include the following:
- App Suggestions:
- `OSC 9001; SuggestInput; <cmd>; <description>` will be used by the attached CLI app to directly make suggestions to the terminal. The TermControl layer will stuff the `cmd` parameter into a new entry in `Suggestions`. An optional `description` can be provided to display in the UI for additional information.
- Triggers and Pattern Matchers[^4] (Similar to VS Code's Terminal[^1])
- when `git push` fails due to an upstream not being set, suggest to push with the upstream set.
- When a `git` subcommand fails with a similar command error, suggest to use the similar command(s).
- When `git push` results in a suggestion to create a GitHub PR, suggest to open the link.
As more sources of suggestions are added, `IVector<String> GetQuickFixContext(UInt32 Row)` will be modified to return `IVector<QuickFixSuggestion>` instead, where the `QuickFixSuggestion` will be able to include additional metadata like the optional description above, an icon, or anything we feel is necessary in the future.

This paradigm can be extended to gutter marks for shell integration as well.
## Resources
[^1]: [VS Code: Terminal Quick Fix Documentation](https://code.visualstudio.com/docs/terminal/shell-integration#_quick-fixes)
[^2]: [VS Code: Expose shell integration command knowledge to extensions](https://github.com/microsoft/vscode/issues/145234)
[^3]: [VS Code: Terminal quick fix API](https://github.com/microsoft/vscode/issues/162950)
[^4]: [Triggers and Custom Clickable Links Spec](https://github.com/microsoft/terminal/pull/15700)