Skip to content

Conversation

e82eric
Copy link
Contributor

@e82eric e82eric commented Jul 16, 2025

Summary of the Pull Request

Adds scroll back source to suggestions

There were 2 things that are different about this from my original screenshot in the feature request.

  • In the feature request I had removed the MaxWidth on the _descriptionsBackdrop so that the control would expand for larger words. This helped for file paths, I decided to remove this change in case it affects the UX of the other sources.
  • In the feature request I had changed the color of the highlights to VsCode blue. When I tried this on a windows 10 machine I realized that the default accent color is blueish and the highlights were no longer visible so I reverted back to not setting the color.

I am not 100% sure on the performance of this. When I list out all of the paths in my home dir and search, the UI stutters in debug mode but doesn't in the release build. (I tried this on a really slow windows 10 machine with the release build and couldn't see any delay).

  • I think listing and searching all the paths in my home dir is close to the worst case scenario, there is a chance that the regex is set to something that brings back every cell as a word, which would cause there to be millions of words but they would be fast to search over.
suggestions.mp4

References and Relevant Issues

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

PR Checklist

winrt::hstring currentWorkingDirectory;
winrt::hstring filter;

bool sortResults = source == SuggestionsSource::Scrollback;
Copy link
Contributor Author

@e82eric e82eric Jul 16, 2025

Choose a reason for hiding this comment

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

For the scroll back source having matches with the highest score and then the ordinal sort first felt like it had the best UX.

{
// Reverse the list
std::reverse(std::begin(actions), std::end(actions));
constexpr std::size_t MaxResults = 1000;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if this is ok, when I was profiling this with 36k results, it was bothering me that the sort was using the same amount of CPU as the fuzzy search.

//300 is the max height of the listView, this may need to be adjusted to include the description height and gap.
const bool estimateCanOpenDownwards = (_anchor.Y + 300) < space.Height;
_setDirection(estimateCanOpenDownwards ? TerminalApp::SuggestionsDirection::TopDown :
TerminalApp::SuggestionsDirection::BottomUp);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this, I was running into a issue where the first time I would open the control from bottom up, the results wouldn't be sorted in reverse, it looked like this was because canOpenDownwards was being calculated after the sort. I used the hard coded 300 because I think canOpenDownward depends on the items being set in the control.

Style="{StaticResource NoAnimationsPlease}" />
Style="{StaticResource NoAnimationsPlease}">
<ListView.ItemContainerTransitions>
<TransitionCollection/>
Copy link
Contributor Author

@e82eric e82eric Jul 16, 2025

Choose a reason for hiding this comment

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

Not sure if this is ok, I removed the content transition.

  • On Windows 11 when I searched it seemed like _scrollIntoView didn't do anything and the selected item would drift to the middle of the results, unless I used ReplaceAll when setting the items.
  • On Windows 10 when I used ReplaceAll the animations were extremely noisy
    ** Removing the content transitions caused the noise to go away.

I didn't notice any difference when using the other sources on both Windows 10 and Windows 11 after I did this but I may not have tested it through enough.

@@ -67,6 +67,12 @@ namespace Microsoft.Terminal.Control
Boolean SearchRegexInvalid;
};

struct SuggestionSearchItem
{
String Row;
Copy link
Contributor Author

@e82eric e82eric Jul 16, 2025

Choose a reason for hiding this comment

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

This copies the entire row contents for every word for the control Description, this was really useful for context. An alternative to copying this would be to set the RowNumber here and have the control ask for the line text each time an item is selected.

This is what that change would look like. 29cec5b

@e82eric e82eric marked this pull request as ready for review July 16, 2025 06:54
@e82eric e82eric force-pushed the scrollback_suggestions_squash branch from 798dd09 to 23984a0 Compare July 17, 2025 01:14
@e82eric e82eric force-pushed the scrollback_suggestions_squash branch from 23984a0 to 12c0a48 Compare July 18, 2025 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestions: Add scroll back as suggestion source.
1 participant