Skip to content

Introduce PowerShell installer stub #18639

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

Open
wants to merge 7 commits into
base: dev/cazamor/sui/extensions-page
Choose a base branch
from
Open

Introduce PowerShell installer stub #18639

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2e7e37a
Introduce PowerShell installer stub
carlos-zamora Feb 28, 2025
a7d6e27
apply feedback
carlos-zamora Apr 17, 2025
cc1d632
fix ARM64
carlos-zamora Apr 18, 2025
0fe444e
add feature flag
carlos-zamora Apr 18, 2025
36d28e2
fix rebase
carlos-zamora Apr 21, 2025
2445500
Merge branch 'dev/cazamor/sui/extensions-page' into dev/cazamor/sui/e…
carlos-zamora Apr 29, 2025
8db1805
Merge branch 'dev/cazamor/sui/extensions-page' into dev/cazamor/sui/e…
carlos-zamora May 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/cascadia/TerminalApp/TabManagement.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ namespace winrt::TerminalApp::implementation
}
const auto settings{ TerminalSettings::CreateWithNewTerminalArgs(_settings, newTerminalArgs, *_bindings) };

if (profile.Source() == L"Windows.Terminal.InstallPowerShell")
{
TraceLoggingWrite(
g_hTerminalAppProvider,
"InstallPowerShellStubInvoked",
TraceLoggingDescription("Event emitted when the 'Install Latest PowerShell' stub was invoked"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
}

// Try to handle auto-elevation
if (_maybeElevate(newTerminalArgs, settings, profile))
{
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalSettingsEditor/Extensions.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@
x:Uid="Extensions_NavigateToProfileButton"
Click="NavigateToProfile_Click"
Style="{StaticResource SettingContainerResetButtonStyle}"
Tag="{x:Bind Profile.Guid}">
Tag="{x:Bind Profile.Guid}"
Visibility="{x:Bind mtu:Converters.InvertedBooleanToVisibility(Profile.Deleted)}">
<FontIcon Glyph="&#xE8A7;"
Style="{StaticResource SettingContainerFontIconStyle}" />
</Button>
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/AzureCloudShellGenerator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ std::wstring_view AzureCloudShellGenerator::GetIcon() const noexcept
// - <none>
// Return Value:
// - a vector with the Azure Cloud Shell connection profile, if available.
void AzureCloudShellGenerator::GenerateProfiles(std::vector<winrt::com_ptr<implementation::Profile>>& profiles) const
void AzureCloudShellGenerator::GenerateProfiles(std::vector<winrt::com_ptr<implementation::Profile>>& profiles)
{
if (AzureConnection::IsAzureConnectionAvailable())
{
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/AzureCloudShellGenerator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model
std::wstring_view GetNamespace() const noexcept override;
std::wstring_view GetDisplayName() const noexcept override;
std::wstring_view GetIcon() const noexcept override;
void GenerateProfiles(std::vector<winrt::com_ptr<implementation::Profile>>& profiles) const override;
void GenerateProfiles(std::vector<winrt::com_ptr<implementation::Profile>>& profiles) override;
};
};
14 changes: 7 additions & 7 deletions src/cascadia/TerminalSettingsModel/CascadiaSettings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
void _appendProfile(winrt::com_ptr<Profile>&& profile, const winrt::guid& guid, ParsedSettings& settings);
void _addUserProfileParent(const winrt::com_ptr<implementation::Profile>& profile);
bool _addOrMergeUserColorScheme(const winrt::com_ptr<implementation::ColorScheme>& colorScheme);
void _executeGenerator(const IDynamicProfileGenerator& generator);
void _executeGenerator(IDynamicProfileGenerator& generator);
void _cleanupPowerShellInstaller(bool isPowerShellInstalled);
winrt::com_ptr<implementation::ExtensionPackage> _registerFragment(const winrt::Microsoft::Terminal::Settings::Model::FragmentSettings& fragment, FragmentScope scope);

std::unordered_set<winrt::hstring, til::transparent_hstring_hash, til::transparent_hstring_equal_to> _ignoredNamespaces;
Expand Down Expand Up @@ -235,14 +236,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
public:
FragmentProfileEntry(winrt::guid profileGuid, hstring json) :
_profileGuid{ profileGuid },
_json{ json } {}
Json{ json } {}

winrt::guid ProfileGuid() const noexcept { return _profileGuid; }
hstring Json() const noexcept { return _json; }
til::property<hstring> Json;

private:
winrt::guid _profileGuid;
hstring _json;
};

struct FragmentColorSchemeEntry : FragmentColorSchemeEntryT<FragmentColorSchemeEntry>
Expand All @@ -266,27 +266,27 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
public:
FragmentSettings(hstring source, hstring json, hstring filename) :
_source{ source },
_json{ json },
_Json{ json },
_filename{ filename } {}

hstring Source() const noexcept { return _source; }
hstring Json() const noexcept { return _json; }
hstring Filename() const noexcept { return _filename; }
Windows::Foundation::Collections::IVector<Model::FragmentProfileEntry> ModifiedProfiles() const noexcept { return _modifiedProfiles; }
void ModifiedProfiles(const Windows::Foundation::Collections::IVector<Model::FragmentProfileEntry>& modifiedProfiles) noexcept { _modifiedProfiles = modifiedProfiles; }
Windows::Foundation::Collections::IVector<Model::FragmentProfileEntry> NewProfiles() const noexcept { return _newProfiles; }
void NewProfiles(const Windows::Foundation::Collections::IVector<Model::FragmentProfileEntry>& newProfiles) noexcept { _newProfiles = newProfiles; }
Windows::Foundation::Collections::IVector<Model::FragmentColorSchemeEntry> ColorSchemes() const noexcept { return _colorSchemes; }
void ColorSchemes(const Windows::Foundation::Collections::IVector<Model::FragmentColorSchemeEntry>& colorSchemes) noexcept { _colorSchemes = colorSchemes; }
WINRT_PROPERTY(hstring, Json);

public:
// views
Windows::Foundation::Collections::IVectorView<Model::FragmentProfileEntry> ModifiedProfilesView() const noexcept { return _modifiedProfiles ? _modifiedProfiles.GetView() : nullptr; }
Windows::Foundation::Collections::IVectorView<Model::FragmentProfileEntry> NewProfilesView() const noexcept { return _newProfiles ? _newProfiles.GetView() : nullptr; }
Windows::Foundation::Collections::IVectorView<Model::FragmentColorSchemeEntry> ColorSchemesView() const noexcept { return _colorSchemes ? _colorSchemes.GetView() : nullptr; }

private:
hstring _source;
hstring _json;
hstring _filename;
Windows::Foundation::Collections::IVector<Model::FragmentProfileEntry> _modifiedProfiles;
Windows::Foundation::Collections::IVector<Model::FragmentProfileEntry> _newProfiles;
Expand Down
94 changes: 87 additions & 7 deletions src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#if TIL_FEATURE_DYNAMICSSHPROFILES_ENABLED
#include "SshHostGenerator.h"
#endif
#include "PowershellInstallationProfileGenerator.h"

#include "ApplicationState.h"
#include "DefaultTerminal.h"
Expand Down Expand Up @@ -178,15 +179,90 @@ SettingsLoader::SettingsLoader(const std::string_view& userJSON, const std::stri
// (meaning profiles specified by the application rather by the user).
void SettingsLoader::GenerateProfiles()
{
_executeGenerator(PowershellCoreProfileGenerator{});
_executeGenerator(WslDistroGenerator{});
_executeGenerator(AzureCloudShellGenerator{});
_executeGenerator(VisualStudioGenerator{});
{
PowershellCoreProfileGenerator powerShellGenerator{};
_executeGenerator(powerShellGenerator);

if (Feature_PowerShellInstallerProfileGenerator::IsEnabled())
{
const auto isPowerShellInstalled = !powerShellGenerator.GetPowerShellInstances().empty();
if (!isPowerShellInstalled)
{
// Only generate the installer stub profile if PowerShell isn't installed.
PowershellInstallationProfileGenerator pwshInstallationGenerator{};
_executeGenerator(pwshInstallationGenerator);
}

// Regardless of running the installer's generator, we need to do some cleanup still.
_cleanupPowerShellInstaller(isPowerShellInstalled);
}
}

WslDistroGenerator wslGenerator{};
_executeGenerator(wslGenerator);

AzureCloudShellGenerator acsGenerator{};
_executeGenerator(acsGenerator);

VisualStudioGenerator vsGenerator{};
_executeGenerator(vsGenerator);
Copy link
Member

Choose a reason for hiding this comment

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

Is it really necessary to extend the lifetime of these generators? They only get destroyed at the end of the scope now.

Copy link
Member Author

Choose a reason for hiding this comment

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

What motivated this change was that the PowershellCoreProfileGenerator already does all the work of finding the existing PowerShell instances; it uses that information to generate (and configure) the appropriate amount of PowerShell profiles.

We need an instance of the profile generator lying around so that we can reuse that information. That way, if we detect that there's an instance of PowerShell installed, we hide the installer stub.

Admittedly, it's not ideal, but its all in one function. Any ideas on how to get around it? Is it worth making the PowershellCoreProfileGenerator special? Or maybe I should pull out the GetPowerShellInstances() code into a static function and undo the work of keeping the generators around (I'll give that a try).


#if TIL_FEATURE_DYNAMICSSHPROFILES_ENABLED
_executeGenerator(SshHostGenerator{});
SshHostGenerator sshGenerator{};
_executeGenerator(sshGenerator);
#endif
}

// Retrieve the "Install Latest PowerShell" profile and...
// - add a comment to the JSON to indicate it's conditionally applied
// - (if PowerShell is installed) mark it for deletion
void SettingsLoader::_cleanupPowerShellInstaller(bool isPowerShellInstalled)
{
const hstring pwshInstallerNamespace{ PowershellInstallationProfileGenerator::Namespace };
if (extensionPackageMap.contains(pwshInstallerNamespace))
{
if (const auto& fragExtList = extensionPackageMap[pwshInstallerNamespace]->Fragments(); fragExtList.Size() > 0)
{
Json::StreamWriterBuilder styledWriter;
styledWriter["indentation"] = " ";
styledWriter["commentStyle"] = "All";

auto fragExt = get_self<FragmentSettings>(fragExtList.GetAt(0));

// We want the comment to be the first thing in the object,
// "closeOnExit" is the first property, so target that.
auto fragExtJson = _parseJSON(til::u16u8(fragExt->Json()));
Copy link
Member

Choose a reason for hiding this comment

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

To be honest, I kind of forgot how the extension code works, but this still seems kind of hacky to me.

The fragment in this case is based on an extension, right? Don't extensions generate profiles only? I'm missing the big picture how we go from profiles to JSON here, hence my confusion. Basically, since extensions generate profiles (including PowershellInstallationProfileGenerator), my assumption would be that this function doesn't need to deal with parsing JSON at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh this section is SUPER hacky! The sole purpose of this is to add the JSON comment so it appears like this:
image
image

So yeah, we have to parse the JSON, add the comment, then store the new JSON with the comment back in.
We have to do it twice:

  1. the settings.json blob representing the entire fragment extension
  2. the generated profile itself

Copy link
Member Author

Choose a reason for hiding this comment

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

Figured it'd be good to try and tell the user that this profile only appears if PowerShell is not installed. It's a weird little behavior that I figured at least a JSON comment would help point out, even if not many people read the JSON comment.

fragExtJson[JsonKey(ProfilesKey)][0]["closeOnExit"].setComment(til::u16u8(fmt::format(FMT_COMPILE(L"// {}"), RS_(L"PowerShellInstallationProfileJsonComment"))), Json::CommentPlacement::commentBefore);
fragExt->Json(hstring{ til::u8u16(Json::writeString(styledWriter, fragExtJson)) });

if (const auto& profileEntryList = fragExt->NewProfilesView(); profileEntryList.Size() > 0)
{
auto profileEntry = get_self<FragmentProfileEntry>(profileEntryList.GetAt(0));

// We want the comment to be the first thing in the object,
// "closeOnExit" is the first property, so target that.
auto profileJson = _parseJSON(til::u16u8(profileEntry->Json()));
profileJson["closeOnExit"].setComment(til::u16u8(fmt::format(FMT_COMPILE(L"// {}"), RS_(L"PowerShellInstallationProfileJsonComment"))), Json::CommentPlacement::commentBefore);
profileEntry->Json(hstring{ til::u8u16(Json::writeString(styledWriter, profileJson)) });

// If PowerShell is installed, mark the installer profile for deletion
if (isPowerShellInstalled)
{
const auto profileGuid = profileEntryList.GetAt(0).ProfileGuid();
for (const auto& profile : userSettings.profiles)
{
if (profile->Guid() == profileGuid)
{
profile->Deleted(true);
break;
}
}
}
}
}
}
}

// A new settings.json gets a special treatment:
// 1. The default profile is a PowerShell 7+ one, if one was generated,
// and falls back to the standard PowerShell 5 profile otherwise.
Expand Down Expand Up @@ -994,7 +1070,7 @@ bool SettingsLoader::_addOrMergeUserColorScheme(const winrt::com_ptr<implementat

// As the name implies it executes a generator.
// Generated profiles are added to .inboxSettings. Used by GenerateProfiles().
void SettingsLoader::_executeGenerator(const IDynamicProfileGenerator& generator)
void SettingsLoader::_executeGenerator(IDynamicProfileGenerator& generator)
{
const auto generatorNamespace = generator.GetNamespace();
std::vector<winrt::com_ptr<implementation::Profile>> generatedProfiles;
Expand Down Expand Up @@ -1588,7 +1664,11 @@ void CascadiaSettings::_resolveNewTabMenuProfiles() const
auto activeProfileCount = gsl::narrow_cast<int>(_activeProfiles.Size());
for (auto profileIndex = 0; profileIndex < activeProfileCount; profileIndex++)
{
remainingProfilesMap.emplace(profileIndex, _activeProfiles.GetAt(profileIndex));
const auto& profile = _activeProfiles.GetAt(profileIndex);
if (!profile.Deleted())
{
remainingProfilesMap.emplace(profileIndex, _activeProfiles.GetAt(profileIndex));
}
}

// We keep track of the "remaining profiles" - those that have not yet been resolved
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/IDynamicProfileGenerator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model
virtual std::wstring_view GetNamespace() const noexcept = 0;
virtual std::wstring_view GetDisplayName() const noexcept = 0;
virtual std::wstring_view GetIcon() const noexcept = 0;
virtual void GenerateProfiles(std::vector<winrt::com_ptr<implementation::Profile>>& profiles) const = 0;
virtual void GenerateProfiles(std::vector<winrt::com_ptr<implementation::Profile>>& profiles) = 0;
};
};
2 changes: 2 additions & 0 deletions src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
<DependentUpon>KeyChordSerialization.idl</DependentUpon>
</ClInclude>
<ClInclude Include="PowershellCoreProfileGenerator.h" />
<ClInclude Include="PowershellInstallationProfileGenerator.h" />
<ClInclude Include="Profile.h">
<DependentUpon>Profile.idl</DependentUpon>
</ClInclude>
Expand Down Expand Up @@ -167,6 +168,7 @@
<DependentUpon>KeyChordSerialization.idl</DependentUpon>
</ClCompile>
<ClCompile Include="PowershellCoreProfileGenerator.cpp" />
<ClCompile Include="PowershellInstallationProfileGenerator.cpp" />
<ClCompile Include="Profile.cpp">
<DependentUpon>Profile.idl</DependentUpon>
</ClCompile>
Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/TerminalSettingsModel/Microsoft.Terminal.Settings.ModelLib.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
<ClCompile Include="PowershellCoreProfileGenerator.cpp">
<Filter>profileGeneration</Filter>
</ClCompile>
<ClCompile Include="PowershellInstallationProfileGenerator.cpp">
<Filter>profileGeneration</Filter>
</ClCompile>
<ClCompile Include="WslDistroGenerator.cpp">
<Filter>profileGeneration</Filter>
</ClCompile>
Expand Down Expand Up @@ -57,6 +60,9 @@
<ClInclude Include="PowershellCoreProfileGenerator.h">
<Filter>profileGeneration</Filter>
</ClInclude>
<ClInclude Include="PowershellInstallationProfileGenerator.h">
<Filter>profileGeneration</Filter>
</ClInclude>
<ClInclude Include="WslDistroGenerator.h">
<Filter>profileGeneration</Filter>
</ClInclude>
Expand Down
Loading
Loading