VsSetup: Replace com_ptr params with raw pointers, clean up code (#11242)

This commit aligns the COM-consuming code in VsSetupInstance with best
practices such as passing COM pointers by pointer when they do not need
to be owning references and not using `const` on members, as well as
cleans up some dead code.

Leonard contributed clang-tidy fixes and some reference passing
changes.

Co-authored-by: Leonard Hecker <lhecker@microsoft.com>
This commit is contained in:
Dustin L. Howett 2021-09-15 19:41:49 -05:00 committed by GitHub
parent f84da18d1e
commit 6983ecf1ba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 80 additions and 97 deletions

View file

@ -14,18 +14,16 @@ std::vector<Profile> BaseVisualStudioGenerator::GenerateProfiles()
// There's no point in enumerating valid Visual Studio instances more than once,
// so cache them for use by both Visual Studio profile generators.
if (!BaseVisualStudioGenerator::hasQueried)
{
instances = VsSetupConfiguration::QueryInstances();
hasQueried = true;
}
static const auto instances = VsSetupConfiguration::QueryInstances();
for (auto const& instance : BaseVisualStudioGenerator::instances)
for (auto const& instance : instances)
{
try
{
if (!IsInstanceValid(instance))
{
continue;
}
auto DevShell{ CreateProfile(GetProfileGuidSeed(instance)) };
DevShell.Name(GetProfileName(instance));

View file

@ -22,20 +22,19 @@ namespace Microsoft::Terminal::Settings::Model
{
class BaseVisualStudioGenerator : public IDynamicProfileGenerator
{
virtual bool IsInstanceValid(const VsSetupConfiguration::VsSetupInstance instance) const = 0;
virtual std::wstring GetProfileName(const VsSetupConfiguration::VsSetupInstance instance) const = 0;
virtual std::wstring GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance instance) const = 0;
virtual std::wstring GetProfileGuidSeed(const VsSetupConfiguration::VsSetupInstance instance) const = 0;
virtual std::wstring GetProfileIconPath() const = 0;
public:
// Inherited via IDynamicProfileGenerator
virtual std::wstring_view GetNamespace() override = 0;
std::wstring_view GetNamespace() override = 0;
std::vector<winrt::Microsoft::Terminal::Settings::Model::Profile> GenerateProfiles() override;
private:
inline static bool hasQueried = false;
inline static std::vector<VsSetupConfiguration::VsSetupInstance> instances;
protected:
virtual bool IsInstanceValid(const VsSetupConfiguration::VsSetupInstance& instance) const = 0;
virtual std::wstring GetProfileName(const VsSetupConfiguration::VsSetupInstance& instance) const = 0;
virtual std::wstring GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance& instance) const = 0;
virtual std::wstring GetProfileGuidSeed(const VsSetupConfiguration::VsSetupInstance& instance) const = 0;
virtual std::wstring GetProfileIconPath() const = 0;
private:
winrt::Microsoft::Terminal::Settings::Model::Profile CreateProfile(const std::wstring_view instanceId);
};
};

View file

@ -6,14 +6,14 @@
using namespace Microsoft::Terminal::Settings::Model;
std::wstring VsDevCmdGenerator::GetProfileName(const VsSetupConfiguration::VsSetupInstance instance) const
std::wstring VsDevCmdGenerator::GetProfileName(const VsSetupConfiguration::VsSetupInstance& instance) const
{
std::wstring name{ L"Developer Command Prompt for VS " };
name.append(instance.GetProfileNameSuffix());
return name;
}
std::wstring VsDevCmdGenerator::GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance instance) const
std::wstring VsDevCmdGenerator::GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance& instance) const
{
std::wstring commandLine{ L"cmd.exe /k \"" + instance.GetDevCmdScriptPath() + L"\"" };

View file

@ -21,15 +21,12 @@ namespace Microsoft::Terminal::Settings::Model
class VsDevCmdGenerator : public BaseVisualStudioGenerator
{
public:
VsDevCmdGenerator() = default;
~VsDevCmdGenerator() = default;
std::wstring_view GetNamespace() override
{
return std::wstring_view{ L"Windows.Terminal.VisualStudio.CommandPrompt" };
}
inline bool IsInstanceValid(const VsSetupConfiguration::VsSetupInstance instance) const override
bool IsInstanceValid(const VsSetupConfiguration::VsSetupInstance&) const override
{
// We only support version of VS from 15.0.
// Per heaths: The [ISetupConfiguration] COM server only supports Visual Studio 15.0 and newer anyway.
@ -37,17 +34,17 @@ namespace Microsoft::Terminal::Settings::Model
return true;
}
inline std::wstring GetProfileGuidSeed(const VsSetupConfiguration::VsSetupInstance instance) const override
std::wstring GetProfileGuidSeed(const VsSetupConfiguration::VsSetupInstance& instance) const override
{
return L"VsDevCmd" + instance.GetInstanceId();
}
inline std::wstring GetProfileIconPath() const override
std::wstring GetProfileIconPath() const override
{
return L"ms-appx:///ProfileIcons/{0caa0dad-35be-5f56-a8ff-afceeeaa6101}.png";
}
std::wstring GetProfileName(const VsSetupConfiguration::VsSetupInstance instance) const override;
std::wstring GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance instance) const override;
std::wstring GetProfileName(const VsSetupConfiguration::VsSetupInstance& instance) const override;
std::wstring GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance& instance) const override;
};
};

View file

@ -3,20 +3,18 @@
#include "pch.h"
#include "VsDevShellGenerator.h"
#include "Setup.Configuration.h"
#include "DefaultProfileUtils.h"
#include "VsSetupConfiguration.h"
using namespace Microsoft::Terminal::Settings::Model;
std::wstring VsDevShellGenerator::GetProfileName(const VsSetupConfiguration::VsSetupInstance instance) const
std::wstring VsDevShellGenerator::GetProfileName(const VsSetupConfiguration::VsSetupInstance& instance) const
{
std::wstring name{ L"Developer PowerShell for VS " };
name.append(instance.GetProfileNameSuffix());
return name;
}
std::wstring VsDevShellGenerator::GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance instance) const
std::wstring VsDevShellGenerator::GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance& instance) const
{
// The triple-quotes are a PowerShell path escape sequence that can safely be stored in a JSON object.
// The "SkipAutomaticLocation" parameter will prevent "Enter-VsDevShell" from automatically setting the shell path

View file

@ -21,30 +21,27 @@ namespace Microsoft::Terminal::Settings::Model
class VsDevShellGenerator : public BaseVisualStudioGenerator
{
public:
VsDevShellGenerator() = default;
~VsDevShellGenerator() = default;
std::wstring_view GetNamespace() override
{
return std::wstring_view{ L"Windows.Terminal.VisualStudio.Powershell" };
}
inline bool IsInstanceValid(const VsSetupConfiguration::VsSetupInstance instance) const override
bool IsInstanceValid(const VsSetupConfiguration::VsSetupInstance& instance) const override
{
return instance.VersionInRange(L"[16.2,)");
}
inline std::wstring GetProfileGuidSeed(const VsSetupConfiguration::VsSetupInstance instance) const override
std::wstring GetProfileGuidSeed(const VsSetupConfiguration::VsSetupInstance& instance) const override
{
return L"VsDevShell" + instance.GetInstanceId();
}
inline std::wstring GetProfileIconPath() const override
std::wstring GetProfileIconPath() const override
{
return L"ms-appx:///ProfileIcons/{61c54bbd-c2c6-5271-96e7-009a87ff44bf}.png";
}
std::wstring GetProfileName(const VsSetupConfiguration::VsSetupInstance instance) const override;
std::wstring GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance instance) const override;
std::wstring GetProfileName(const VsSetupConfiguration::VsSetupInstance& instance) const override;
std::wstring GetProfileCommandLine(const VsSetupConfiguration::VsSetupInstance& instance) const override;
};
};

View file

@ -8,7 +8,7 @@ using namespace Microsoft::Terminal::Settings::Model;
std::vector<VsSetupConfiguration::VsSetupInstance> VsSetupConfiguration::QueryInstances()
{
std::vector<VsSetupConfiguration::VsSetupInstance> instances;
std::vector<VsSetupInstance> instances;
// SetupConfiguration is only registered if Visual Studio is installed
ComPtrSetupQuery pQuery{ wil::CoCreateInstanceNoThrow<SetupConfiguration, ISetupConfiguration2>() };
@ -21,13 +21,13 @@ std::vector<VsSetupConfiguration::VsSetupInstance> VsSetupConfiguration::QueryIn
wil::com_ptr<IEnumSetupInstances> e;
THROW_IF_FAILED(pQuery->EnumInstances(&e));
std::array<ComPtrSetupInstance, 1> rgpInstance;
auto result = e->Next(1, &rgpInstance[0], NULL);
ComPtrSetupInstance rgpInstance;
auto result = e->Next(1, &rgpInstance, nullptr);
while (S_OK == result)
{
// wrap the COM pointers in a friendly interface
instances.emplace_back(VsSetupConfiguration::VsSetupInstance{ pQuery, rgpInstance[0] });
result = e->Next(1, &rgpInstance[0], NULL);
instances.emplace_back(VsSetupInstance{ pQuery, rgpInstance });
result = e->Next(1, &rgpInstance, nullptr);
}
THROW_IF_FAILED(result);
@ -38,7 +38,7 @@ std::vector<VsSetupConfiguration::VsSetupInstance> VsSetupConfiguration::QueryIn
/// <summary>
/// Takes a relative path under a Visual Studio installation and returns the absolute path.
/// </summary>
std::wstring VsSetupConfiguration::ResolvePath(ComPtrSetupInstance pInst, std::wstring_view relativePath)
std::wstring VsSetupConfiguration::ResolvePath(ISetupInstance* pInst, std::wstring_view relativePath)
{
wil::unique_bstr bstrAbsolutePath;
THROW_IF_FAILED(pInst->ResolvePath(relativePath.data(), &bstrAbsolutePath));
@ -49,9 +49,9 @@ std::wstring VsSetupConfiguration::ResolvePath(ComPtrSetupInstance pInst, std::w
/// Determines whether a Visual Studio installation version falls within a specified range.
/// The range is specified as a string, ex: "[15.0.0.0,)", "[15.0.0.0, 16.7.0.0)
/// </summary>
bool VsSetupConfiguration::InstallationVersionInRange(ComPtrSetupQuery pQuery, ComPtrSetupInstance pInst, std::wstring_view range)
bool VsSetupConfiguration::InstallationVersionInRange(ISetupConfiguration2* pQuery, ISetupInstance* pInst, std::wstring_view range)
{
auto helper = pQuery.query<ISetupHelper>();
const auto helper = wil::com_query<ISetupHelper>(pQuery);
// VS versions in a string format such as "16.3.0.0" can be easily compared
// by parsing them into 64-bit unsigned integers using the stable algorithm
@ -70,14 +70,14 @@ bool VsSetupConfiguration::InstallationVersionInRange(ComPtrSetupQuery pQuery, C
return ullVersion >= minVersion && ullVersion <= maxVersion;
}
std::wstring VsSetupConfiguration::GetInstallationVersion(ComPtrSetupInstance pInst)
std::wstring VsSetupConfiguration::GetInstallationVersion(ISetupInstance* pInst)
{
wil::unique_bstr bstrInstallationVersion;
THROW_IF_FAILED(pInst->GetInstallationVersion(&bstrInstallationVersion));
return bstrInstallationVersion.get();
}
std::wstring VsSetupConfiguration::GetInstallationPath(ComPtrSetupInstance pInst)
std::wstring VsSetupConfiguration::GetInstallationPath(ISetupInstance* pInst)
{
wil::unique_bstr bstrInstallationPath;
THROW_IF_FAILED(pInst->GetInstallationPath(&bstrInstallationPath));
@ -88,14 +88,14 @@ std::wstring VsSetupConfiguration::GetInstallationPath(ComPtrSetupInstance pInst
/// The instance id is unique for each Visual Studio installation on a system.
/// The instance id is generated by the Visual Studio setup engine and varies from system to system.
/// </summary>
std::wstring VsSetupConfiguration::GetInstanceId(ComPtrSetupInstance pInst)
std::wstring VsSetupConfiguration::GetInstanceId(ISetupInstance* pInst)
{
wil::unique_bstr bstrInstanceId;
THROW_IF_FAILED(pInst->GetInstanceId(&bstrInstanceId));
return bstrInstanceId.get();
}
std::wstring VsSetupConfiguration::GetStringProperty(ComPtrPropertyStore pProps, std::wstring_view name)
std::wstring VsSetupConfiguration::GetStringProperty(ISetupPropertyStore* pProps, std::wstring_view name)
{
if (pProps == nullptr)
{

View file

@ -24,7 +24,6 @@ namespace Microsoft::Terminal::Settings::Model
/// </summary>
class VsSetupConfiguration
{
private:
typedef wil::com_ptr<ISetupConfiguration2> ComPtrSetupQuery;
typedef wil::com_ptr<ISetupHelper> ComPtrSetupHelper;
typedef wil::com_ptr<ISetupInstance> ComPtrSetupInstance;
@ -38,12 +37,12 @@ namespace Microsoft::Terminal::Settings::Model
public:
struct VsSetupInstance
{
inline std::wstring ResolvePath(std::wstring_view relativePath) const
std::wstring ResolvePath(std::wstring_view relativePath) const
{
return VsSetupConfiguration::ResolvePath(inst, relativePath);
return VsSetupConfiguration::ResolvePath(inst.get(), relativePath);
}
inline std::wstring GetDevShellModulePath() const
std::wstring GetDevShellModulePath() const
{
// The path of Microsoft.VisualStudio.DevShell.dll changed in 16.3
if (VersionInRange(L"[16.3,"))
@ -54,39 +53,39 @@ namespace Microsoft::Terminal::Settings::Model
return ResolvePath(L"Common7\\Tools\\vsdevshell\\Microsoft.VisualStudio.DevShell.dll");
}
inline std::wstring GetDevCmdScriptPath() const
std::wstring GetDevCmdScriptPath() const
{
return ResolvePath(L"Common7\\Tools\\VsDevCmd.bat");
}
inline bool VersionInRange(std::wstring_view range) const
bool VersionInRange(std::wstring_view range) const
{
return VsSetupConfiguration::InstallationVersionInRange(query, inst, range);
return InstallationVersionInRange(query.get(), inst.get(), range);
}
inline std::wstring GetVersion() const
std::wstring GetVersion() const
{
return VsSetupConfiguration::GetInstallationVersion(inst);
return GetInstallationVersion(inst.get());
}
inline std::wstring GetInstallationPath() const
std::wstring GetInstallationPath() const
{
return VsSetupConfiguration::GetInstallationPath(inst);
return VsSetupConfiguration::GetInstallationPath(inst.get());
}
inline std::wstring GetInstanceId() const
std::wstring GetInstanceId() const
{
return VsSetupConfiguration::GetInstanceId(inst);
return VsSetupConfiguration::GetInstanceId(inst.get());
}
inline ComPtrPropertyStore GetInstancePropertyStore() const
ComPtrPropertyStore GetInstancePropertyStore() const
{
ComPtrPropertyStore properties;
inst.query_to<ISetupPropertyStore>(&properties);
return properties;
}
inline ComPtrCustomPropertyStore GetCustomPropertyStore() const
ComPtrCustomPropertyStore GetCustomPropertyStore() const
{
ComPtrSetupInstance2 instance2;
inst.query_to<ISetupInstance2>(&instance2);
@ -99,7 +98,7 @@ namespace Microsoft::Terminal::Settings::Model
return properties;
}
inline ComPtrCatalogPropertyStore GetCatalogPropertyStore() const
ComPtrCatalogPropertyStore GetCatalogPropertyStore() const
{
ComPtrInstanceCatalog instanceCatalog;
inst.query_to<ISetupInstanceCatalog>(&instanceCatalog);
@ -112,7 +111,7 @@ namespace Microsoft::Terminal::Settings::Model
return properties;
}
inline std::wstring GetProfileNameSuffix() const
std::wstring GetProfileNameSuffix() const
{
return profileNameSuffix;
}
@ -120,34 +119,32 @@ namespace Microsoft::Terminal::Settings::Model
private:
friend class VsSetupConfiguration;
VsSetupInstance(const ComPtrSetupQuery pQuery, const ComPtrSetupInstance pInstance) :
query(pQuery),
helper(pQuery.query<ISetupHelper>()),
inst(pInstance),
VsSetupInstance(ComPtrSetupQuery pQuery, ComPtrSetupInstance pInstance) :
query(std::move(pQuery)),
inst(std::move(pInstance)),
profileNameSuffix(BuildProfileNameSuffix())
{
}
const ComPtrSetupQuery query;
const ComPtrSetupHelper helper;
const ComPtrSetupInstance inst;
ComPtrSetupQuery query;
ComPtrSetupInstance inst;
std::wstring profileNameSuffix;
inline std::wstring BuildProfileNameSuffix() const
std::wstring BuildProfileNameSuffix() const
{
ComPtrCatalogPropertyStore catalogProperties = GetCatalogPropertyStore();
if (catalogProperties != nullptr)
{
std::wstring suffix;
std::wstring productLine{ GetProductLineVersion(catalogProperties) };
std::wstring productLine{ GetProductLineVersion(catalogProperties.get()) };
suffix.append(productLine);
ComPtrCustomPropertyStore customProperties = GetCustomPropertyStore();
if (customProperties != nullptr)
{
std::wstring nickname{ GetNickname(customProperties) };
std::wstring nickname{ GetNickname(customProperties.get()) };
if (!nickname.empty())
{
suffix.append(L" (" + nickname + L")");
@ -155,13 +152,13 @@ namespace Microsoft::Terminal::Settings::Model
else
{
ComPtrPropertyStore instanceProperties = GetInstancePropertyStore();
suffix.append(GetChannelNameSuffixTag(instanceProperties));
suffix.append(GetChannelNameSuffixTag(instanceProperties.get()));
}
}
else
{
ComPtrPropertyStore instanceProperties = GetInstancePropertyStore();
suffix.append(GetChannelNameSuffixTag(instanceProperties));
suffix.append(GetChannelNameSuffixTag(instanceProperties.get()));
}
return suffix;
@ -170,7 +167,7 @@ namespace Microsoft::Terminal::Settings::Model
return GetVersion();
}
inline std::wstring GetChannelNameSuffixTag(ComPtrPropertyStore instanceProperties) const
static std::wstring GetChannelNameSuffixTag(ISetupPropertyStore* instanceProperties)
{
std::wstring tag;
std::wstring channelName{ GetChannelName(instanceProperties) };
@ -188,12 +185,12 @@ namespace Microsoft::Terminal::Settings::Model
return tag;
}
inline std::wstring GetChannelId(ComPtrPropertyStore instanceProperties) const
static std::wstring GetChannelId(ISetupPropertyStore* instanceProperties)
{
return VsSetupConfiguration::GetStringProperty(instanceProperties, L"channelId");
return GetStringProperty(instanceProperties, L"channelId");
}
inline std::wstring GetChannelName(ComPtrPropertyStore instanceProperties) const
static std::wstring GetChannelName(ISetupPropertyStore* instanceProperties)
{
std::wstring channelId{ GetChannelId(instanceProperties) };
if (channelId.empty())
@ -213,28 +210,25 @@ namespace Microsoft::Terminal::Settings::Model
return channelName;
}
inline std::wstring GetNickname(ComPtrCustomPropertyStore customProperties) const
static std::wstring GetNickname(ISetupPropertyStore* customProperties)
{
return VsSetupConfiguration::GetStringProperty(customProperties, L"nickname");
return GetStringProperty(customProperties, L"nickname");
}
inline std::wstring GetProductLineVersion(ComPtrCatalogPropertyStore customProperties) const
static std::wstring GetProductLineVersion(ISetupPropertyStore* customProperties)
{
return VsSetupConfiguration::GetStringProperty(customProperties, L"productLineVersion");
return GetStringProperty(customProperties, L"productLineVersion");
}
};
static std::vector<VsSetupConfiguration::VsSetupInstance> QueryInstances();
static std::vector<VsSetupInstance> QueryInstances();
private:
VsSetupConfiguration() = default;
~VsSetupConfiguration() = default;
static bool InstallationVersionInRange(ComPtrSetupQuery pQuery, ComPtrSetupInstance pInst, std::wstring_view range);
static std::wstring ResolvePath(ComPtrSetupInstance pInst, std::wstring_view relativePath);
static std::wstring GetInstallationVersion(ComPtrSetupInstance pInst);
static std::wstring GetInstallationPath(ComPtrSetupInstance pInst);
static std::wstring GetInstanceId(ComPtrSetupInstance pInst);
static std::wstring GetStringProperty(ComPtrPropertyStore pProps, std::wstring_view name);
static bool InstallationVersionInRange(ISetupConfiguration2* pQuery, ISetupInstance* pInst, std::wstring_view range);
static std::wstring ResolvePath(ISetupInstance* pInst, std::wstring_view relativePath);
static std::wstring GetInstallationVersion(ISetupInstance* pInst);
static std::wstring GetInstallationPath(ISetupInstance* pInst);
static std::wstring GetInstanceId(ISetupInstance* pInst);
static std::wstring GetStringProperty(ISetupPropertyStore* pProps, std::wstring_view name);
};
};