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.

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

[Peek] Fix Monaco assets folder discovery #35925

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

daverayment
Copy link
Contributor

Summary of the Pull Request

The MonacoHelper class in FilePreviewCommon handles locating the Monaco assets directory so the Monaco language file and template HTML file may be loaded. This PR corrects an issue whereby GetRuntimeMonacoDirectory() would return an incorrect folder location if an empty INSTALLDIR\WinUI3Apps\Assets\Monaco existed in addition to the correct INSTALLDIR\Assets\Monaco location.

PR Checklist

  • Closes: peek bug #35728
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

The function now does an explicit check to see if the calling assembly lies within the WinUI3Apps folder, and if so it navigates to the parent directory before continuing resolution of the assets path.

Validation Steps Performed

I reproduced the failure case by creating an empty \AppData\Local\PowerToys\WinUI3Apps\Assets\Monaco folder and then using Peek to preview files supported by Monaco, such as source code files. This confirmed the original report. After applying the fix and retesting, the same Peek previews correctly rendered the files.

I also confirmed that the Explorer Preview Pane was unaffected, and also used Peek to view several non-Monaco files, such as images and PDFs, to confirm that other file types were still rendering without issue.

Before and After

image

image

Additional Info

A WinUI3Apps\Assets\Monaco folder is created when compiling the solution from source, and its contents look identical to the folder a level up in the hierarchy. I don't know if this is related to the original issue, but thought it was worth a mention.

@crutkas crutkas added the Needs-Review This Pull Request awaits the review of a maintainer. label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Review This Pull Request awaits the review of a maintainer.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants