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

Clean up windows association manager code #31450

Merged
merged 3 commits into from
Jan 8, 2025
Merged

Clean up windows association manager code #31450

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d04947d
Don't use `record`s they are ugly
peppy Jan 8, 2025
b628880
Change association localisation flow to make logical sense
peppy Jan 8, 2025
e131a6c
Add explicit `ToString()` to avoid sending `LocalisableString` to reg…
peppy Jan 8, 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
87 changes: 51 additions & 36 deletions osu.Desktop/Windows/WindowsAssociationManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,13 @@ public static class WindowsAssociationManager
/// Installs file and URI associations.
/// </summary>
/// <remarks>
/// Call <see cref="UpdateDescriptions"/> in a timely fashion to keep descriptions up-to-date and localised.
/// Call <see cref="LocaliseDescriptions"/> in a timely fashion to keep descriptions up-to-date and localised.
/// </remarks>
public static void InstallAssociations()
{
try
{
updateAssociations();
updateDescriptions(null); // write default descriptions in case `UpdateDescriptions()` is not called.
NotifyShellUpdate();
}
catch (Exception e)
Expand All @@ -76,17 +75,13 @@ public static void InstallAssociations()
/// Updates associations with latest definitions.
/// </summary>
/// <remarks>
/// Call <see cref="UpdateDescriptions"/> in a timely fashion to keep descriptions up-to-date and localised.
/// Call <see cref="LocaliseDescriptions"/> in a timely fashion to keep descriptions up-to-date and localised.
/// </remarks>
public static void UpdateAssociations()
{
try
{
updateAssociations();

// TODO: Remove once UpdateDescriptions() is called as specified in the xmldoc.
updateDescriptions(null); // always write default descriptions, in case of updating from an older version in which file associations were not implemented/installed

NotifyShellUpdate();
}
catch (Exception e)
Expand All @@ -95,11 +90,17 @@ public static void UpdateAssociations()
}
}

public static void UpdateDescriptions(LocalisationManager localisationManager)
// TODO: call this sometime.
public static void LocaliseDescriptions(LocalisationManager localisationManager)
{
try
{
updateDescriptions(localisationManager);
foreach (var association in file_associations)
association.LocaliseDescription(localisationManager);

foreach (var association in uri_associations)
association.LocaliseDescription(localisationManager);

NotifyShellUpdate();
}
catch (Exception e)
Expand Down Expand Up @@ -140,17 +141,6 @@ private static void updateAssociations()
association.Install();
}

private static void updateDescriptions(LocalisationManager? localisation)
{
foreach (var association in file_associations)
association.UpdateDescription(getLocalisedString(association.Description));

foreach (var association in uri_associations)
association.UpdateDescription(getLocalisedString(association.Description));

string getLocalisedString(LocalisableString s) => localisation?.GetLocalisedString(s) ?? s.ToString();
}

#region Native interop

[DllImport("Shell32.dll")]
Expand All @@ -174,9 +164,20 @@ private enum Flags : uint

#endregion

private record FileAssociation(string Extension, LocalisableString Description, string IconPath)
private class FileAssociation
{
private string programId => $@"{program_id_prefix}{Extension}";
private string programId => $@"{program_id_prefix}{extension}";

private string extension { get; }
private LocalisableString description { get; }
private string iconPath { get; }

public FileAssociation(string extension, LocalisableString description, string iconPath)
{
this.extension = extension;
this.description = description;
this.iconPath = iconPath;
}

/// <summary>
/// Installs a file extension association in accordance with https://learn.microsoft.com/en-us/windows/win32/com/-progid--key
Expand All @@ -189,14 +190,16 @@ public void Install()
// register a program id for the given extension
using (var programKey = classes.CreateSubKey(programId))
{
programKey.SetValue(null, description.ToString());

using (var defaultIconKey = programKey.CreateSubKey(default_icon))
defaultIconKey.SetValue(null, IconPath);
defaultIconKey.SetValue(null, iconPath);

using (var openCommandKey = programKey.CreateSubKey(SHELL_OPEN_COMMAND))
openCommandKey.SetValue(null, $@"""{exe_path}"" ""%1""");
}

using (var extensionKey = classes.CreateSubKey(Extension))
using (var extensionKey = classes.CreateSubKey(extension))
{
// set ourselves as the default program
extensionKey.SetValue(null, programId);
Expand All @@ -208,13 +211,13 @@ public void Install()
}
}

public void UpdateDescription(string description)
public void LocaliseDescription(LocalisationManager localisationManager)
{
using var classes = Registry.CurrentUser.OpenSubKey(software_classes, true);
if (classes == null) return;

using (var programKey = classes.OpenSubKey(programId, true))
programKey?.SetValue(null, description);
programKey?.SetValue(null, localisationManager.GetLocalisedString(description));
}

/// <summary>
Expand All @@ -225,7 +228,7 @@ public void Uninstall()
using var classes = Registry.CurrentUser.OpenSubKey(software_classes, true);
if (classes == null) return;

using (var extensionKey = classes.OpenSubKey(Extension, true))
using (var extensionKey = classes.OpenSubKey(extension, true))
{
// clear our default association so that Explorer doesn't show the raw programId to users
// the null/(Default) entry is used for both ProdID association and as a fallback friendly name, for legacy reasons
Expand All @@ -240,13 +243,24 @@ public void Uninstall()
}
}

private record UriAssociation(string Protocol, LocalisableString Description, string IconPath)
private class UriAssociation
{
/// <summary>
/// "The <c>URL Protocol</c> string value indicates that this key declares a custom pluggable protocol handler."
/// See https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa767914(v=vs.85).
/// </summary>
public const string URL_PROTOCOL = @"URL Protocol";
private const string url_protocol = @"URL Protocol";

private string protocol { get; }
private LocalisableString description { get; }
private string iconPath { get; }

public UriAssociation(string protocol, LocalisableString description, string iconPath)
{
this.protocol = protocol;
this.description = description;
this.iconPath = iconPath;
}

/// <summary>
/// Registers an URI protocol handler in accordance with https://learn.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa767914(v=vs.85).
Expand All @@ -256,31 +270,32 @@ public void Install()
using var classes = Registry.CurrentUser.OpenSubKey(software_classes, true);
if (classes == null) return;

using (var protocolKey = classes.CreateSubKey(Protocol))
using (var protocolKey = classes.CreateSubKey(protocol))
{
protocolKey.SetValue(URL_PROTOCOL, string.Empty);
protocolKey.SetValue(null, $@"URL:{description}");
Copy link
Member Author

Choose a reason for hiding this comment

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

Default that was previously written in UpdateDescription(null) is now written here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's with the URL: prefix in the string? That looks new?

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't be, copied from https://github.com/ppy/osu/pull/31450/files#diff-3014968ab29090e493772996bc2000d02214e9e009cf3f27325f85020cc0b0abL277

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh that is hidden away, I completely failed to spot it...... Sure yeah.

protocolKey.SetValue(url_protocol, string.Empty);

using (var defaultIconKey = protocolKey.CreateSubKey(default_icon))
defaultIconKey.SetValue(null, IconPath);
defaultIconKey.SetValue(null, iconPath);

using (var openCommandKey = protocolKey.CreateSubKey(SHELL_OPEN_COMMAND))
openCommandKey.SetValue(null, $@"""{exe_path}"" ""%1""");
}
}

public void UpdateDescription(string description)
public void LocaliseDescription(LocalisationManager localisationManager)
{
using var classes = Registry.CurrentUser.OpenSubKey(software_classes, true);
if (classes == null) return;

using (var protocolKey = classes.OpenSubKey(Protocol, true))
protocolKey?.SetValue(null, $@"URL:{description}");
using (var protocolKey = classes.OpenSubKey(protocol, true))
protocolKey?.SetValue(null, $@"URL:{localisationManager.GetLocalisedString(description)}");
}

public void Uninstall()
{
using var classes = Registry.CurrentUser.OpenSubKey(software_classes, true);
classes?.DeleteSubKeyTree(Protocol, throwOnMissingSubKey: false);
classes?.DeleteSubKeyTree(protocol, throwOnMissingSubKey: false);
}
}
}
Expand Down
Loading