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.

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

Add support for "Tasks" in the Suggestions UI #15664

Merged
merged 141 commits into from
Aug 15, 2023
Merged

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented Jul 6, 2023

targets #15027

Adds a new suggestion source, tasks, that allows a user to open the Suggestions UI with sendInput commands saved in their settings. source becomes a flag setting, so it can be combined like so:

        {
            "keys": "ctrl+shift+h", "command": { "action": "suggestions", "source": "commandHistory", "useCommandline":true },
        },
        {
            "keys": "ctrl+shift+y", "command": { "action": "suggestions", "source": "tasks", "useCommandline":false },
        },
        {
            "keys": "ctrl+shift+b", "command": { "action": "suggestions", "source": ["all"], "useCommandline":true },
        },

If a nested command has sendInput commands underneath it, this will build a tree of commands that only include sendInputs as leaves (but leave the rest of the nesting structure intact).

References and Relevant Issues

Closes #1595

See also #13445
As spec'd in #14864

Validation Steps Performed

Tested manually

zadjii-msft and others added 30 commits April 28, 2022 16:14
…Ev-DaYs' into dev/migrie/fhl-2023/pwsh-autocomplete-demo
auto base{ RS_(L"SuggestionsCommandKey") };
switch (Source())
std::wstringstream ss;
ss << RS_(L"SuggestionsCommandKey").c_str();
Copy link
Member

Choose a reason for hiding this comment

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

nit: std::wstring_view{ RS_(L"SuggestionsCommandKey") }.
You can also just use std::wstring::append() instead of using wstringsteam, because string streams are fairly hefty.

Copy link
Member

Choose a reason for hiding this comment

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

looks like we merged with the ss and the comment unresolved 😄

src/cascadia/TerminalSettingsModel/ActionArgs.cpp Outdated Show resolved Hide resolved
Copy link
Member

@lhecker lhecker left a comment

Choose a reason for hiding this comment

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

✅ with nits.

src/cascadia/TerminalApp/AppActionHandlers.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/ActionMap.cpp Outdated Show resolved Hide resolved
Comment on lines +69 to +74
_subcommands = winrt::single_threaded_map<winrt::hstring, Model::Command>();

for (const auto& n : nested)
{
_subcommands.Insert(n.Name(), n);
}
Copy link
Member

Choose a reason for hiding this comment

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

Oh, I forgot to mark this one for the std::vector vs. IVector thing

Base automatically changed from dev/migrie/f/sxnui-useCommandline to main August 15, 2023 15:39
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, this is the VS version. With the trailing spaces. Which is insane.

@zadjii-msft zadjii-msft enabled auto-merge (squash) August 15, 2023 17:58
@microsoft-github-policy-service microsoft-github-policy-service bot added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Aug 15, 2023
winrt::hstring{ fmt::format(FMT_COMPILE(L"{:\x7f^{}}{}"),
L"",
numBackspaces,
(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.

BTW does this really work? I didn't think strict ternary operators allowed this. I thought it would have needed to be something like this:

inArgs ? std::wstring_view{ inArgs.Input() } : std::wstring_view{}

so that each arm of the ternary has the exact same type.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seemed like it compiled and ran fine? Now that I look at it, it does feel weird that it compiles. I feel like usually the compiler is much more strict about this kind of thing...

Copy link
Member

Choose a reason for hiding this comment

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

I think it's because the hstring gets implicitly turned into a C string. The string-view constructor will then do a strlen to turn it back. That's not particularly optimal, but we also won't die from that. 😅

@zadjii-msft zadjii-msft merged commit 127df07 into main Aug 15, 2023
15 of 17 checks passed
@zadjii-msft zadjii-msft deleted the dev/migrie/f/1595-tasks branch August 15, 2023 19:01
// then get that here.
const bool shouldGetContext = realArgs.UseCommandline() ||
WI_IsFlagSet(source, SuggestionsSource::CommandHistory);
if (shouldGetContext)
{
if (const auto& control{ _GetActiveControl() })
Copy link
Member

Choose a reason for hiding this comment

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

Hey, didn't we make a change on your other suggestions PR to use sender instead of active control?

auto base{ RS_(L"SuggestionsCommandKey") };
switch (Source())
std::wstringstream ss;
ss << RS_(L"SuggestionsCommandKey").c_str();
Copy link
Member

Choose a reason for hiding this comment

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

looks like we merged with the ss and the comment unresolved 😄

@@ -931,4 +931,72 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
return _ExpandedCommandsCache;
}

IVector<Model::Command> _filterToSendInput(IMapView<hstring, Model::Command> nameMap,
Copy link
Member

Choose a reason for hiding this comment

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

rude! this is not filtering, this is constructing a completely different set of commands. that's a different operation than filtering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. zBugBash-Consider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request - Scripts Panel
4 participants