Preview Handler for developer files #15138

Aaron-Junker · 2021-12-25T07:16:25Z

Summary of the Pull Request

What is this about:

Adds the preview handler for developer files. I open this PR, as integration is planned for january release (0.55). Here can things regarding the integration be discussed.

What needs to be done:

Clint's list

Add the font to signing
Sanity check this is a .NET 5 project
Remove Newtson and swap to System. JSON like all other libs
Swap to our common.ui theme system in 0.55 & removing the system.preview from signing
- This would need to be a 0.56 item otherwise
Add 0.56 add unit test item (maybe this should just mimic maybe what the markdown previewer unit tests do?) (
Add unit tests for developer preview #15749)
Test on blank VM to be sure installer installs for Webview2 runtime
Update notice.md

Aaron's list:

Quality Checklist

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

htcfreek · 2022-01-25T16:49:30Z

@Aaron-Junker
Why not foreach when reading json. I see no reason to use a for loop.

Sory for not commenting in code. GitHub app can't load the file list. It's to big. 😅

Aaron-Junker · 2022-01-25T16:51:55Z

@Aaron-Junker Why not foreach when reading json. I see no reason to use a for loop.

Sory for not commenting in code. GitHub app can't load the file list. It's to big. 😅

I didn't knew foreach existed in C#. I thought this was a PHP only thing😅.

Will adress this for the next release.

htcfreek · 2022-01-25T16:56:45Z

@Aaron-Junker Why not foreach when reading json. I see no reason to use a for loop.

Sory for not commenting in code. GitHub app can't load the file list. It's to big. 😅

I didn't knew foreach existed in C#. I thought this was a PHP only thing😅.

Will adress this for the next release.

You can do something like for element in json. But don't ask me for the correct syntax. It isn't in my brain at the moment. 😅

stefansjfw

I took another look, but basically I already did a review while integrating monaco into powertoys. I found few minor issues. Would be nice to solve right now, but non-blocking imho

EDIT : Please don't merge, wait for at least one more approve

stefansjfw · 2022-01-25T16:50:44Z

src/modules/previewpane/MonacoPreviewHandler/FileHandler.cs

+        {
+            try
+            {
+                JsonDocument a = JsonDocument.Parse(File.ReadAllText(Settings.AssemblyDirectory + "\\languages.json"));


nit: a not a very descriptive var name..

Forgot this placeholder 😅

stefansjfw · 2022-01-25T16:53:08Z

src/modules/previewpane/MonacoPreviewHandler/MonacoPreviewHandlerControl.cs

+        /// This event cancels every navigation inside the webview
+        /// </summary>
+        [STAThread]
+        private void NavigationStarted(object sender, CoreWebView2NavigationStartingEventArgs e)


this is never called?

Technically it shpuld be registered as eventhandler. But I'm not 100% sure

…Pane

jaimecbernardo

Left some comments.
After building and installing, I can't seem to open settings;

Description: A .NET Core application failed.
Application: PowerToys.Settings.exe
Path: C:\Program Files\PowerToys\Settings\PowerToys.Settings.exe
Message: Error:
  An assembly specified in the application dependencies manifest (PowerToys.Settings.deps.json) was not found:
    package: 'Microsoft.Web.WebView2.Core', version: '255.255.255.255'
    path: 'Microsoft.Web.WebView2.Core.winmd'

I'm assuming this was caused by #15707

jaimecbernardo · 2022-01-25T17:31:45Z

.pipelines/versionAndSignCheck.ps1

@@ -7,7 +7,7 @@ Param(
 )

 $DirPath = $targetDir;  #this file is in pipeline, we need root.
-$items = Get-ChildItem -Path $DirPath -File -Include *.exe,*.dll -Recurse -Force -ErrorAction SilentlyContinue
+$items = Get-ChildItem -Path $DirPath -File -Include *.exe,*.dll,*.ttf -Recurse -Force -ErrorAction SilentlyContinue


Do ttf files need to be signed?

@crutkas said yes. He will look into it in a future PR

Should they be OTF instead?

jaimecbernardo · 2022-01-25T17:36:32Z

installer/PowerToysSetup/Product.wxs

@@ -127,7 +128,7 @@
            <Custom Action="TelemetryLogUninstallSuccess" After="InstallFinalize">
                Installed and (NOT UPGRADINGPRODUCTCODE) AND (REMOVE="ALL")
            </Custom>
-            <Custom Action="UnApplyModulesRegistryChangeSets" Before="InstallFinalize">
+            <Custom Action="UnApplyModulesRegistryChangeSets" Before="RemoveFiles">


Why did this change?

I think because we need some of the files we then remove in the next step.

@htcfreek is correct, to unapply the monaco handler registry changes from CA installer, we need to read languages.json file to get a list of extensions the handler is used for.

yuyoyuppe

I've been mostly focused on installer/registry-related changes, and they look good to me.

This reverts commit b6a207c.

This reverts commit 1a25dac.

crutkas · 2022-01-25T18:15:13Z

@jaimecbernardo that is saying settings is requesting WebView2 which shouldn't be needed for settings app

stefansjfw · 2022-01-25T18:17:22Z

@jaimecbernardo that is saying settings is requesting WebView2 which shouldn't be needed for settings app

I think it's related to what @niels9001 did with bumping versions.. I'll revert those.. We can deal with it later

htcfreek · 2022-01-25T18:23:59Z

@jaimecbernardo that is saying settings is requesting WebView2 which shouldn't be needed for settings app

Maybe it's this change in v2.8: microsoft/microsoft-ui-xaml#5929

crutkas · 2022-01-25T18:24:58Z

once @jaimecbernardo / @stefansjfw approve, the merge can be all yours. Great job

stefansjfw · 2022-01-25T18:27:12Z

@jaimecbernardo I pushed revert commits, can you give it another tests to doublecheck it? After that , if everything is ok, It's good to go IMHO

jaimecbernardo

LGTM now!
Great work here!
Thanks a lot for this!
Feel free to squash merge @Aaron-Junker .

crutkas · 2022-01-25T19:04:54Z

🚀🚀🚀🔥🔥🔥🔥🚀🚀🚀🚀🚀🚀🔥🔥🔥🔥🔥

🚢🚢🚢🚢🚢

stefansjfw · 2022-01-25T20:39:31Z

Niiiiceee! 🎉🎉

htcfreek · 2022-01-25T21:04:20Z

Coooool. It's ready to use now. 🚀👏

chip33 · 2022-02-01T13:30:29Z

Just trying this with v0.55.0.
I am wondering why it does not support commonly used scripting files in windows such as .vbs , .ahk, .au3. Is there a settings file where we can add these new filetypes?
Also some filetypes such as .reg and many of "PerceivedType"="text" show in the Preview pane with a white background when using a dark theme. Is there a setting to change this?

Really fantastic work to achieve this. It has been missing in Windows for so long.

Aaron-Junker · 2022-02-01T13:39:15Z

Just trying this with v0.55.0.
I am wondering why it does not support commonly used scripting files in windows such as .vbs , .ahk, .au3. Is there a settings file where we can add these new filetypes?
Also some filetypes such as .reg and many of "PerceivedType"="text" show in the Preview pane with a white background when using a dark theme. Is there a setting to change this?

Currently there's not an easy way to add costum types.
I've added your suggestion to #14957. What do you mean with "PerceivedType"="text"?

Really fantastic work to achieve this. It has been missing in Windows for so long.

Thank you!

chip33 · 2022-02-01T14:16:18Z

Thanks for adding the request!
"PerceivedType"="text" is a registry setting applied natively to many filetypes.

[HKEY_CLASSES_ROOT\.text] "PerceivedType"="text"

or

[HKEY_CLASSES_ROOT\.sql] "PerceivedType"="text"

Filetypes of PerceivedType=text always show in the Preview pane so It was often recommended to add PerceivedType=text to a filetype as a hacky workaround to make it visible in the Preview pane.

With Preview Handler for developer files .sql shows with syntax highlighting as a named filetype in your code but .text and many others of PerceivedType=text still show as the default Windows white preview pane.
It would be good if they could also show the same dark preview as filetype .txt.

Aaron-Junker · 2022-02-01T14:25:41Z

Thanks for adding the request!
"PerceivedType"="text" is a registry setting applied natively to many filetypes.

[HKEY_CLASSES_ROOT\.text] "PerceivedType"="text"

or

[HKEY_CLASSES_ROOT\.sql] "PerceivedType"="text"

Filetypes of PerceivedType=text always show in the Preview pane so It was often recommended to add PerceivedType=text to a filetype as a hacky workaround to make it visible in the Preview pane.

With Preview Handler for developer files .sql shows with syntax highlighting as a named filetype in your code but .text and many others of PerceivedType=text still show as the default Windows white preview pane.
It would be good if they could also show the same dark preview as filetype .txt.

You could manually copy the registry key of a file type that uses the new design to another one.

crutkas and others added 30 commits September 2, 2021 17:02

Create Readme.md

f3d42f2

Update Readme.md

a9beb26

Rebased to master

a614ee8

started integrating

d43ac24

Resolve merge conflict

130f516

Fixed merge conflict

7ed7d1f

Edited expect.txt

41386e8

Tried implementig interfaces

6bc8349

Push

c5a91a4

Push

652d71b

push

4d37cca

push

ca01669

Deleted no longer used files

d22dde6

push

b9c439d

push

16b4e9f

Thanks @jaimecbernando for helping

f0d578e

can load via url

086fcd8

getting stuff semi stubbed out

a60106a

full render without passing vars

9052fd9

making sure we clearly label the file was us

5c12139

push

f863081

fix merge conflicts

2011894

push

e9adb70

push (does not work)

d859290

Push

048d98c

push

7696c72

Added file size limit of 3 Kilobyte in standalone App (MoncaoPreview)

d68e0db

Added monacosrc to excludes.txt (spell checker)

6589ab1

Fixed XAMLHost loading issue and changed namespace of Settings.cs

9f4c79b

push

8780026

Remove Newtonsoft.Json.dll from installer

5395f00

stefansjfw approved these changes Jan 25, 2022

View reviewed changes

Merge remote-tracking branch 'origin/main' into feature/monacoPreview…

fff9c0a

…Pane

jaimecbernardo requested changes Jan 25, 2022

View reviewed changes

yuyoyuppe approved these changes Jan 25, 2022

View reviewed changes

stefansjfw added 2 commits January 25, 2022 19:14

Revert "WinUI bump (#15707)"

11e4dc2

This reverts commit b6a207c.

Revert "Update Settings.UI.csproj (#15704)"

e265f0a

This reverts commit 1a25dac.

crutkas requested a review from stefansjfw January 25, 2022 18:24

jaimecbernardo approved these changes Jan 25, 2022

View reviewed changes

Aaron-Junker merged commit 39b98da into main Jan 25, 2022

Aaron-Junker deleted the feature/monacoPreviewPane branch January 25, 2022 20:02

This was referenced Jan 25, 2022

[PowerRename] WinUI bump #15707

Merged

[Settings] Nuget packages update #15704

Merged

[Settings] Plugin search field: Text and symbol overlap #13080

Closed

[meta]git ignore monaco generated installer file #15763

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preview Handler for developer files #15138

Preview Handler for developer files #15138

Aaron-Junker commented Dec 25, 2021 •

edited by stefansjfw

Loading

htcfreek commented Jan 25, 2022 •

edited

Loading

Aaron-Junker commented Jan 25, 2022 •

edited

Loading

htcfreek commented Jan 25, 2022

stefansjfw left a comment •

edited

Loading

stefansjfw Jan 25, 2022

Aaron-Junker Jan 25, 2022

stefansjfw Jan 25, 2022

Aaron-Junker Jan 25, 2022 •

edited

Loading

jaimecbernardo left a comment

jaimecbernardo Jan 25, 2022

Aaron-Junker Jan 25, 2022

jsoref Jan 25, 2022

jaimecbernardo Jan 25, 2022

htcfreek Jan 25, 2022

yuyoyuppe Jan 25, 2022

yuyoyuppe left a comment •

edited

Loading

crutkas commented Jan 25, 2022

stefansjfw commented Jan 25, 2022

htcfreek commented Jan 25, 2022

crutkas commented Jan 25, 2022

stefansjfw commented Jan 25, 2022

jaimecbernardo left a comment

crutkas commented Jan 25, 2022

stefansjfw commented Jan 25, 2022

htcfreek commented Jan 25, 2022

chip33 commented Feb 1, 2022

Aaron-Junker commented Feb 1, 2022

chip33 commented Feb 1, 2022

Aaron-Junker commented Feb 1, 2022

Preview Handler for developer files #15138

Preview Handler for developer files #15138

Conversation

Aaron-Junker commented Dec 25, 2021 • edited by stefansjfw Loading

Summary of the Pull Request

Clint's list

Aaron's list:

Quality Checklist

Contributor License Agreement (CLA)

htcfreek commented Jan 25, 2022 • edited Loading

Aaron-Junker commented Jan 25, 2022 • edited Loading

htcfreek commented Jan 25, 2022

stefansjfw left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aaron-Junker Jan 25, 2022 • edited Loading

Choose a reason for hiding this comment

jaimecbernardo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuyoyuppe left a comment • edited Loading

Choose a reason for hiding this comment

crutkas commented Jan 25, 2022

stefansjfw commented Jan 25, 2022

htcfreek commented Jan 25, 2022

crutkas commented Jan 25, 2022

stefansjfw commented Jan 25, 2022

jaimecbernardo left a comment

Choose a reason for hiding this comment

crutkas commented Jan 25, 2022

stefansjfw commented Jan 25, 2022

htcfreek commented Jan 25, 2022

chip33 commented Feb 1, 2022

Aaron-Junker commented Feb 1, 2022

chip33 commented Feb 1, 2022

Aaron-Junker commented Feb 1, 2022

Aaron-Junker commented Dec 25, 2021 •

edited by stefansjfw

Loading

htcfreek commented Jan 25, 2022 •

edited

Loading

Aaron-Junker commented Jan 25, 2022 •

edited

Loading

stefansjfw left a comment •

edited

Loading

Aaron-Junker Jan 25, 2022 •

edited

Loading

yuyoyuppe left a comment •

edited

Loading