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

Remove spaces from snippets in the SXNUI #17261

Merged
merged 10 commits into from
May 14, 2024
Merged

Remove spaces from snippets in the SXNUI #17261

merged 10 commits into from
May 14, 2024

Conversation

zadjii-msft
Copy link
Member

Couple different issues:

  • The suggestions UI can't filter snippets without a name, that have a space in them, because '␣' != ' '. This instead removes the visualized space from the name shown in the SXN UI.
  • Similarly, we generate an action with leading backspaces to remove the current commandline. Then we visualize those BS's as a part of the generated name. Same thing - can't filter to that.

Before in blue:
image
image

closes #16577
closes #16578

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Suggestions Anything related to tasks or the suggestions UI (sxnui) Product-Terminal The new Windows Terminal. labels May 13, 2024
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
lhecker
lhecker approved these changes May 13, 2024
View reviewed changes
src/cascadia/TerminalSettingsModel/ActionMap.cpp Outdated
@@ -964,17 +965,32 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
cmdImpl.copy_from(winrt::get_self<implementation::Command>(command));

const auto inArgs{ command.ActionAndArgs().Args().try_as<Model::SendInputArgs>() };

const auto inputString{ (std::wstring_view)(inArgs ? inArgs.Input() : L"") };
Copy link
Member

Choose a reason for hiding this comment

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

Technically you need to assign the Input() to somewhere before getting a weak string-view to it. It's not guaranteed that the implementation of the property holds onto a reference of the HSTRING after all.

Copy link
Member

Choose a reason for hiding this comment

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

I'm gonna block over this, since it could be an easy safety issue

DHowett
DHowett requested changes May 13, 2024
View reviewed changes
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

as noted

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label May 13, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label May 13, 2024
@DHowett DHowett added this pull request to the merge queue May 14, 2024
Merged via the queue into main with commit 54cfb85 May 14, 2024
20 checks passed
@DHowett DHowett deleted the dev/migrie/b/16577-sxnui-spaces branch May 14, 2024 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DHowett DHowett DHowett approved these changes

@lhecker lhecker lhecker approved these changes

@carlos-zamora carlos-zamora carlos-zamora approved these changes

Assignees

@DHowett DHowett

Labels
Area-Suggestions Anything related to tasks or the suggestions UI (sxnui) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
No milestone
4 participants
@zadjii-msft @DHowett @lhecker @carlos-zamora