Skip to content

Commit

Permalink
[MonacoPreviewHandler][Settings] Resolve preview handler race (#16180)
Browse files Browse the repository at this point in the history
* Register .markdown with the correct handler

* Fix spelling

* Move file name extensions from "expect.txt" to "excludes.txt"

* Revert "Move file name extensions from "expect.txt" to "excludes.txt""

This reverts commit 710d5a4.
I must have misunderstood the instructions.

* Revert "Register .markdown with the correct handler"

This reverts commit 5c37b00.

* Work in progress

* Code ready for testing

* Update excludes.txt

* Update excludes.txt

* Update modulesRegistry.h

* Update modulesRegistry.h

For the want of an exclamation mark, a kingdom is lost!

* Update modulesRegistry.h

* Work on modulesRegistry.h per code review in 16180

Removed all previous exclusions from Monaco preview handler. Added a new exclusion: SVGZ. It's a binary file that Monaco cannot, in any meaningful way, read.

* Update expect.txt

* Update accessory files

* Disable machine-wide checks for performance reasons
  • Loading branch information
skycommand committed Mar 9, 2022
1 parent b415e79 commit 05d5649
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 17 deletions.
1 change: 1 addition & 0 deletions .github/actions/spell-check/excludes.txt
Expand Up @@ -46,6 +46,7 @@ ignore$
^src/modules/previewpane/PreviewPaneUnitTests/HelperFiles/MarkdownWithHTMLImageTag\.txt$
^src/modules/previewpane/UnitTests-MarkdownPreviewHandler/HelperFiles/MarkdownWithHTMLImageTag.txt$
^tools/CleanUp_tool/CleanUp_tool\.vcxproj\.filters$
^tools/Verification scripts/Check preview handler registration\.ps1$
^\.github/
^\.github/actions/spell-check/
^\.gitmodules$
Expand Down
6 changes: 6 additions & 0 deletions .github/actions/spell-check/expect.txt
Expand Up @@ -1179,6 +1179,9 @@ MAXIMIZEBOX
MAXSHORTCUTSIZE
maxversiontested
MBs
mdtext
mdtxt
mdwn
MBUTTON
MBUTTONDBLCLK
MBUTTONDOWN
Expand Down Expand Up @@ -1225,6 +1228,8 @@ Miracast
mirophone
Mishkeegogamang
mjpg
mkd
mkdn
mlcfg
MMDDYYYY
mmdeviceapi
Expand Down Expand Up @@ -1961,6 +1966,7 @@ SVE
SVGIn
SVGIO
svgpreviewhandler
svgz
SWC
SWFO
SWP
Expand Down
53 changes: 37 additions & 16 deletions src/common/utils/modulesRegistry.h
Expand Up @@ -13,8 +13,14 @@ namespace NonLocalizable
const static wchar_t* MONACO_LANGUAGES_FILE_NAME = L"modules\\FileExplorerPreview\\monaco_languages.json";
const static wchar_t* ListID = L"list";
const static wchar_t* ExtensionsID = L"extensions";
const static wchar_t* MDExtension = L".md";
const static wchar_t* SVGExtension = L".svg";
const static std::vector<std::wstring> ExtSVG = { L".svg" };
const static std::vector<std::wstring> ExtMarkdown = { L".md", L".markdown", L".mdown", L".mkdn", L".mkd", L".mdwn", L".mdtxt", L".mdtext" };
const static std::vector<std::wstring> ExtPDF = { L".pdf" };
const static std::vector<std::wstring> ExtGCode = { L".gcode" };
const static std::vector<std::wstring> ExtSTL = { L".stl" };
const static std::vector<std::wstring> ExtNoNoNo = {
L".svgz" //Monaco cannot handle this file type at all; it's a binary file.
};
}

inline registry::ChangeSet getSvgPreviewHandlerChangeSet(const std::wstring installationDir, const bool perUser)
Expand All @@ -30,7 +36,7 @@ inline registry::ChangeSet getSvgPreviewHandlerChangeSet(const std::wstring inst
registry::DOTNET_COMPONENT_CATEGORY_CLSID,
L"Microsoft.PowerToys.PreviewHandler.Svg.SvgPreviewHandler",
L"Svg Preview Handler",
{ L".svg" });
NonLocalizable::ExtSVG);
}

inline registry::ChangeSet getMdPreviewHandlerChangeSet(const std::wstring installationDir, const bool perUser)
Expand All @@ -44,14 +50,23 @@ inline registry::ChangeSet getMdPreviewHandlerChangeSet(const std::wstring insta
registry::DOTNET_COMPONENT_CATEGORY_CLSID,
L"Microsoft.PowerToys.PreviewHandler.Markdown.MarkdownPreviewHandler",
L"Markdown Preview Handler",
{ L".md" });
NonLocalizable::ExtMarkdown);
}

inline registry::ChangeSet getMonacoPreviewHandlerChangeSet(const std::wstring installationDir, const bool perUser)
{
using namespace registry::shellex;

// Set up a list of extensions for the preview handler to take over
std::vector<std::wstring> extensions;

// Set up a list of extensions that Monaco support but the preview handler shouldn't take over
std::vector<std::wstring> ExtExclusions;
ExtExclusions.insert(ExtExclusions.end(), NonLocalizable::ExtMarkdown.begin(), NonLocalizable::ExtMarkdown.end());
ExtExclusions.insert(ExtExclusions.end(), NonLocalizable::ExtSVG.begin(), NonLocalizable::ExtSVG.end());
ExtExclusions.insert(ExtExclusions.end(), NonLocalizable::ExtNoNoNo.begin(), NonLocalizable::ExtNoNoNo.end());
bool IsExcluded = false;

std::wstring languagesFilePath = fs::path{ installationDir } / NonLocalizable::MONACO_LANGUAGES_FILE_NAME;
auto json = json::from_file(languagesFilePath);

Expand All @@ -68,13 +83,19 @@ inline registry::ChangeSet getMonacoPreviewHandlerChangeSet(const std::wstring i
for (uint32_t j = 0; j < extensionsList.Size(); ++j)
{
auto extension = extensionsList.GetStringAt(j);

// Ignore extensions we already have dedicated handlers for
if (std::wstring{ extension } == std::wstring{ NonLocalizable::MDExtension } ||
std::wstring{ extension } == std::wstring{ NonLocalizable::SVGExtension })

// Ignore extensions in the exclusion list
IsExcluded = false;

for (std::wstring k : ExtExclusions)
{
continue;
if (std::wstring{ extension } == k)
{
IsExcluded = true;
break;
}
}
if (IsExcluded) { continue; }
extensions.push_back(std::wstring{ extension });
}
}
Expand Down Expand Up @@ -106,7 +127,7 @@ inline registry::ChangeSet getPdfPreviewHandlerChangeSet(const std::wstring inst
registry::DOTNET_COMPONENT_CATEGORY_CLSID,
L"Microsoft.PowerToys.PreviewHandler.Pdf.PdfPreviewHandler",
L"Pdf Preview Handler",
{ L".pdf" });
NonLocalizable::ExtPDF);
}

inline registry::ChangeSet getGcodePreviewHandlerChangeSet(const std::wstring installationDir, const bool perUser)
Expand All @@ -120,7 +141,7 @@ inline registry::ChangeSet getGcodePreviewHandlerChangeSet(const std::wstring in
registry::DOTNET_COMPONENT_CATEGORY_CLSID,
L"Microsoft.PowerToys.PreviewHandler.Gcode.GcodePreviewHandler",
L"G-code Preview Handler",
{ L".gcode" });
NonLocalizable::ExtGCode);
}

inline registry::ChangeSet getSvgThumbnailHandlerChangeSet(const std::wstring installationDir, const bool perUser)
Expand All @@ -134,7 +155,7 @@ inline registry::ChangeSet getSvgThumbnailHandlerChangeSet(const std::wstring in
registry::DOTNET_COMPONENT_CATEGORY_CLSID,
L"Microsoft.PowerToys.ThumbnailHandler.Svg.SvgThumbnailProvider",
L"Svg Thumbnail Provider",
{ L".svg" });
NonLocalizable::ExtSVG);
}

inline registry::ChangeSet getPdfThumbnailHandlerChangeSet(const std::wstring installationDir, const bool perUser)
Expand All @@ -148,7 +169,7 @@ inline registry::ChangeSet getPdfThumbnailHandlerChangeSet(const std::wstring in
registry::DOTNET_COMPONENT_CATEGORY_CLSID,
L"Microsoft.PowerToys.ThumbnailHandler.Pdf.PdfThumbnailProvider",
L"Pdf Thumbnail Provider",
{ L".pdf" });
NonLocalizable::ExtPDF);
}

inline registry::ChangeSet getGcodeThumbnailHandlerChangeSet(const std::wstring installationDir, const bool perUser)
Expand All @@ -162,7 +183,7 @@ inline registry::ChangeSet getGcodeThumbnailHandlerChangeSet(const std::wstring
registry::DOTNET_COMPONENT_CATEGORY_CLSID,
L"Microsoft.PowerToys.ThumbnailHandler.Gcode.GcodeThumbnailProvider",
L"G-code Thumbnail Provider",
{ L".gcode" });
NonLocalizable::ExtGCode);
}

inline registry::ChangeSet getStlThumbnailHandlerChangeSet(const std::wstring installationDir, const bool perUser)
Expand All @@ -176,7 +197,7 @@ inline registry::ChangeSet getStlThumbnailHandlerChangeSet(const std::wstring in
registry::DOTNET_COMPONENT_CATEGORY_CLSID,
L"Microsoft.PowerToys.ThumbnailHandler.Stl.StlThumbnailProvider",
L"Stl Thumbnail Provider",
{ L".stl" });
NonLocalizable::ExtSTL);
}

inline std::vector<registry::ChangeSet> getAllModulesChangeSets(const std::wstring installationDir)
Expand All @@ -191,4 +212,4 @@ inline std::vector<registry::ChangeSet> getAllModulesChangeSets(const std::wstri
getPdfThumbnailHandlerChangeSet(installationDir, PER_USER),
getGcodeThumbnailHandlerChangeSet(installationDir, PER_USER),
getStlThumbnailHandlerChangeSet(installationDir, PER_USER) };
}
}
2 changes: 1 addition & 1 deletion src/settings-ui/Settings.UI/Strings/en-us/Resources.resw
Expand Up @@ -617,7 +617,7 @@
<value>Show recently used strings</value>
</data>
<data name="FileExplorerPreview_ToggleSwitch_Preview_MD.Header" xml:space="preserve">
<value>Enable Markdown (.md) preview</value>
<value>Enable Markdown preview (.md, .markdown, .mdown, .mkdn, .mkd, .mdwn, .mdtxt, .mdtext)</value>
<comment>Do not loc "Markdown". Do you want this feature on / off</comment>
</data>
<data name="FileExplorerPreview_ToggleSwitch_Preview_Monaco.Header" xml:space="preserve">
Expand Down
76 changes: 76 additions & 0 deletions tools/Verification scripts/Check preview handler registration.ps1
@@ -0,0 +1,76 @@
#Requires -Version 7.2

using namespace System.Management.Automation

[CmdletBinding()]
param()

function PublicStaticVoidMain {
[CmdletBinding()]
param ()

class TypeHandlerData {
[String] $Name
[String] $CurrentUserHandler
[String] $MachineWideHandler
}

[String[]]$TypesToCheck = @(".markdown", ".mdtext", ".mdtxt", ".mdown", ".mkdn", ".mdwn", ".mkd", ".md", ".svg", ".svgz", ".pdf", ".gcode", ".stl", ".txt", ".ini")
$IPREVIEW_HANDLER_CLSID = '{8895b1c6-b41f-4c1c-a562-0d564250836f}'
$PowerToysHandlers = @{
'{07665729-6243-4746-95b7-79579308d1b2}' = "PowerToys PDF handler"
'{ddee2b8a-6807-48a6-bb20-2338174ff779}' = "PowerToys SVG handler"
'{ec52dea8-7c9f-4130-a77b-1737d0418507}' = "PowerToys GCode handler"
'{45769bcc-e8fd-42d0-947e-02beef77a1f5}' = "PowerToys Markdown handler"
'{afbd5a44-2520-4ae0-9224-6cfce8fe4400}' = "PowerToys Monaco fallback handler"
'{DC6EFB56-9CFA-464D-8880-44885D7DC193}' = "Adobe Acrobat DC"
}

function ResolveHandlerGUIDtoName {
param (
[Parameter(Mandatory, Position = 0)]
[String] $GUID
)
return $PowerToysHandlers[$GUID] ?? $GUID
}

function WriteMyProgress {
param (
[Parameter(Mandatory, Position=0)] [Int32] $ItemsPending,
[Parameter(Mandatory, Position=1)] [Int32] $ItemsTotal,
[switch] $Completed
)
[Int32] $PercentComplete = ($ItemsPending / $ItemsTotal) * 100
if ($PercentComplete -lt 1) { $PercentComplete = 1}
Write-Progress -Activity 'Querying Windows Registry' -Status "$ItemsPending of $ItemsTotal" -PercentComplete $PercentComplete -Completed:$Completed
}

$ItemsTotal = $TypesToCheck.Count * 2
$ItemsPending = 0
WriteMyProgress 0 $ItemsTotal

$CheckResults = New-Object -TypeName 'System.Collections.Generic.List[TypeHandlerData]'
foreach ($item in $TypesToCheck) {
$CurrentUserGUID = Get-ItemPropertyValue -Path "HKCU://Software/Classes/$item/shellex/$IPREVIEW_HANDLER_CLSID" -Name '(default)' -ErrorAction SilentlyContinue
$ItemsPending += 1
WriteMyProgress $ItemsPending $ItemsTotal

$MachineWideGUID = "Didn't check"
# $MachineWideGUID = Get-ItemPropertyValue -Path "HKLM://Software/Classes/$item/shellex/$IPREVIEW_HANDLER_CLSID" -Name '(default)' -ErrorAction SilentlyContinue
$ItemsPending += 1
WriteMyProgress $ItemsPending $ItemsTotal

$temp = New-Object -TypeName TypeHandlerData -Property @{
Name = $item
CurrentUserHandler = ($null -eq $CurrentUserGUID) ? "Nothing" : (ResolveHandlerGUIDtoName ($CurrentUserGUID))
MachineWideHandler = ($null -eq $MachineWideGUID) ? "Nothing" : (ResolveHandlerGUIDtoName ($MachineWideGUID))
}
$CheckResults.Add($temp)

Clear-Variable 'CurrentUserGUID', 'MachineWideGUID'
}
WriteMyProgress $ItemsPending $ItemsTotal -Completed
$CheckResults | Format-Table -AutoSize
}

PublicStaticVoidMain

0 comments on commit 05d5649

Please sign in to comment.