[Peek] Add support for previewing AutoHotKey files, CSV, and other plaintext files
This allows for the previewing of AutoHotKey .ahk script files, .csv and .tsv files with the Monaco renderer in Peek (via the WebBrowserPreviewer).
It also introduces an extensible way to add further file extensions for plaintext previews in the future via a simple settings file.
PR Checklist
- [x] Closes: #34483 and #33811
- [x] 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
- [ ] JSON for signing for new binaries
- [ ] WXS for installer for new binaries and localization folder
- [ ] YML for CI pipeline for new test projects
- [ ] YML for signed pipeline
- [ ] 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
This uses Monaco to preview plaintext files even if they are not contained within the Monaco supported languages JSON file.
.ahk, .csv, and .tsv extensions have been added for now. Others may be added (possibly by end-users) by editing the new 'Assets\Monaco\monacoExtraExtensions.txt' file.
A small additional fix was made to MonacoHelper.cs by placing a using for the JsonDocument instantiation there, as it was not Disposed properly previously.
Care has been taken to not override any Shell preview handlers which could render these new file types. (This was necessary because ShellPreviewHandlerPreviewer is later than WebBrowserPreviewer in the IsItemSupported() checks in PreviewerFactory.cs)
It may be worthwhile enhancing this in the future. For example, by:
- Allowing end users to edit the list of plaintext file extensions via Settings; or
- Using file introspection to determine whether a file is plaintext, instead of rejecting it because its extension is unknown and then opening the generic unsupported file previewer.
Validation Steps Performed
- I used the current version of PowerToys to attempt to open .ahk, .csv and .tsv files on my machine, which does not have Microsoft Office installed (so no preview handler for .csv files). I confirmed that the contents were not rendered and the UnsupportedFilePreview was used to show summary information only.
- I confirmed each of the file types could be opened as plaintext with the updated build.
- I ran through my test folder, which contains a mix of image files, documents, text files and so on, confirming there were no issues created for other file types.
Please note: I don't own Microsoft Office / Office 365, so could not confirm that the Excel previewer handler for CSV files still works correctly with this update. Could this please be tested by one of the devs with that software installed? Many thanks.
@check-spelling-bot Report
:red_circle: Please review
See the :open_file_folder: files view, the :scroll:action log, or :memo: job summary for details.
Unrecognized words (2)
ahk tsv
Previously acknowledged words that are now absent
applayout appsfolder cswinrt systemsettings SYSTEMWOW USEPOSITION USESIZE 🫥To accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands
... in a clone of the [email protected]:daverayment/PowerToys.git repository
on the keep-add-ahk branch (:information_source: how do I use this?):
curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.22/apply.pl' |
perl - 'https://github.com/microsoft/PowerToys/actions/runs/10821759987/attempts/1'
Available :books: dictionaries could cover words (expected and unrecognized) not in the :blue_book: dictionary
This includes both expected items (1897) from .github/actions/spell-check/expect.txt and unrecognized words (2)
| Dictionary | Entries | Covers | Uniquely |
|---|---|---|---|
| cspell:r/src/r.txt | 543 | 1 | 1 |
| cspell:cpp/src/people.txt | 23 | 1 | |
| cspell:cpp/src/ecosystem.txt | 51 | 1 |
Consider adding them (in .github/workflows/spelling2.yml) for uses: check-spelling/[email protected] in its with:
with:
extra_dictionaries:
cspell:r/src/r.txt
cspell:cpp/src/people.txt
cspell:cpp/src/ecosystem.txt
To stop checking additional dictionaries, add (in .github/workflows/spelling2.yml) for uses: check-spelling/[email protected] in its with:
check_extra_dictionaries: ''
Warnings (1)
See the :open_file_folder: files view, the :scroll:action log, or :memo: job summary for details.
| :information_source: Warnings | Count |
|---|---|
| :information_source: non-alpha-in-dictionary | 1 |
See :information_source: Event descriptions for more information.
If the flagged items are :exploding_head: false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it, try adding it to the
patterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
@htcfreek Hi. Are you able to add a new file to the spelling checker excludes.txt file, please? The path to the file is \src\common\FilePreviewCommon\Assets\Monaco\monacoExtraExtensions.txt. Thank you!
@daverayment Thank you for the contribution. But unfortunately we can't accept this PR.
- This is the wrong way of implementing this. Please read the docs under https://github.com/microsoft/PowerToys/blob/main/doc%2Fdevdocs%2Fcommon%2FFilePreviewCommon.md .
- You did not ask for implementation of the csv part. This is tricky as it may override the explorer default preview handling and may prevents Excel from handling it.
Thank you for reviewing the PR, @htcfreek 😊
Sorry, I don't think I explained the aims very well.
The intention is to use the Monaco renderer as a plaintext viewer for AutoHotKey scripts and other currently unsupported file extensions. The decision was deliberately made not to add these as new languages to Monaco, or as extensions to existing supported Monaco languages. This has the following advantages:
- Plaintext files do not require syntax highlighting or any other special processing. They are for quick unformatted display only.
- We can add new file extensions to the list much more easily than adding a new language to Monaco.
- If a new language or syntax highlighting for a file type is added to Monaco's supported list, this will be automatically used, superseding the plaintext rendering.
- Users themselves can add their own file extensions to the list without needing us to release a new version of PowerToys. (This is why I didn't hard-code the file extensions and created the new text file instead.)
Essentially, this adds a plaintext preview capability which just uses Monaco as the rendering engine, and I should have made that clearer. Apologies.
I understand that I didn't ask about the CSV or TSV additions. I wanted to provide an example that showed that a file extension could be handled either with a preview handler (for those viewing CSVs with Microsoft Office installed) or with Monaco (for those without Office using this plaintext renderer). There is code within the PR which specifically excludes those extensions which are supported by the ShellHandlerPreviewer, using its IsItemSupported() feature directly.
I still think this is a worthwhile approach to displaying files which do not need special processing. If this PR is unsuitable, I would like to raise it as another issue and perhaps explain it better and open it up to further discussion.
Thanks again.
Why is this blocked?
Why is this blocked?
- Only the changes for AHK are discussed.
- The way we did it till yet is updating the language json in Monaco. But this PR implements a different way to define supported extensions for plain text that never has neen discussed in the issue and allows manual user configuration in a text file.
- The now configuration file is not configurable through setting ui.
I think we should discuss first if we want this.
Would love to see this land!
@crutkas , @Aaron-Junker Can you please look at this PR and discuss internally if you are okay with supporting additional text file extension managed in a settings file by the user. (And without ui in PT Settings.)
this isn't directly a settings file but i do get the feeling. also reason why this isn't a settings file is this would be overwritten every release
this isn't directly a settings file but i do get the feeling. also reason why this isn't a settings file is this would be overwritten every release
So imo, it should definitely be a settings file, so in the future we can also build an UI on top of it. But the bigger problem here is that this only adds support for using it in Peek, as for using it in File Explorer these file extensions would have to be registered additionally in the registry by the settings or the installer. This would require additional logic
Imo this gets handled by PT settings and we provide a default value that the user can extend.
I think we should add the three extensions in the regular way as fallback and open a new issue for the custom extension setting. (At the end the current way of this PR implements something that was never requested and discussed in the issues.)
this isn't directly a settings file but i do get the feeling. also reason why this isn't a settings file is this would be overwritten every release
So imo, it should definitely be a settings file, so in the future we can also build an UI on top of it. But the bigger problem here is that this only adds support for using it in Peek, as for using it in File Explorer these file extensions would have to be registered additionally in the registry by the settings or the installer. This would require additional logic
We do reg stuff for stuff on and off for when we but this does get a little complicated i think. If we shifted to more of a mental model like what #32817 it would unblock the "now" and get these extensions supported in both, no?
I think for a good longer term
https://github.com/microsoft/PowerToys/pull/32817 https://github.com/microsoft/PowerToys/commit/c458087af02e1e6d7040c9248ca2973f68275956#diff-f56a13d2e109105150aaf7bc8ed89dbdc676942801a86bf83c4c42d09f1bcc02L1
@crutkas I am not 100% what yout want to say because your comment seems to miss some words.
But the PR you referenced/adding the extensions to languages.json is the preferred way we do since ever. Doing this is easy and simple and I think for now the best solution to get the preview part of this PR in. What do you think @Aaron-Junker ?
Later and discussed in a new issue we can implement a way for our users to add their own extension for txt files through PT settings.
Thank you everyone for discussing this.
The .txt file for user settings was not a good decision on my part. As @crutkas says, user entries would potentially be overwritten each install. Having an extra 'user-only' file where the user could list their own extensions could be a possibility, but discoverability would be a problem, it would be fragile, and it's all information I accept should really be exposed by an official PowerToys settings property.
I agree that the way forward is to create a new issue for user-configurable plain text file support. I can certainly do that. I'm afraid I'm unfamiliar with the settings system at the moment, but I'll ask if I need a hand.
Likewise, I can edit the monaco_languages.json and monacoSpecialLanguages.js files and hopefully that will cover the AutoHotKey requirement, which was the original issue. You know, before I went and complicated everything 🙄
Do you think it would be better if I closed this off and handled that via another PR?
Thanks again.
@daverayment
- Implementing the user configurable plain text extension in a separate issue and pr sounds good. Please create the issue that we can discuss a good way of implementation. Later we can help with implementation.
- For the implementation of the original issues (.csv, .tsv, .ahk) as plain text using the currently approved way please open a new and clean PR. That easier to review.
- For the language.json part you find a documentation here: https://github.com/microsoft/PowerToys/blob/main/doc%2Fdevdocs%2Fcommon%2FFilePreviewCommon.md