search

Enter-tainer / typst-preview

[DEPRECATED] Use tinymist instead
https://Enter-tainer.github.io/typst-preview/
MIT License
452 stars 21 forks source link

feat: option to ignore system fonts when previewing #299

Closed 7sDream closed 1 month ago

7sDream commented 1 month ago

Another small option which gives more control over compilation environment.

According to typst/typst#4227, the official designation for this argument has been confirmed as --ignore-system-fonts. We follow it in cli argument. In extension, added a typst-preview.systemFonts configuration to control it.

Besides, since there is no clear use case for utilizing VSCode variables within sys.input, this PR disables it.

7sDream commented 1 month ago

BTW, I try run test of addon/vscode locally, but without success, due to 3 blocker:

  1. The test script ./out/test/runTest.js can't be found, seems missing a /src/ path at middle. After I change it to ./out/src/test/runTest.js, the test runs.
  2. @vscode/test-electron can't launch vscode due to microsoft/vscode#200895. Can be fixed by upgrade @vscode/test-electron to 2.4.0.
  3. The VSCode window for testing opens without a workspace, and vscode-variables doesn't handle the case of workspaceFolders is undefined properly.

Since I'm not a heavy user of NodeJS, so unsure if the fix ways above are correct (and It seems that this test is not running in CI?). So I decide to leave fixing the test workflow to someone more suitable.

Enter-tainer commented 1 month ago

The extension test is not well maintained, sorry for that. And it's not running in ci.

7sDream commented 1 month ago

In CLI side, I think --ignore-system-fonts is OK.

But in extension side, I noticed that tinymist uses systemFonts with a default value of true, instead of an inverse switch like no... or ignore.... Should we consistency with it?

Myriad-Dreamin commented 1 month ago

We have different principle of design between configurations and CLI options. For a CLI option, it is natural to make a boolean flag --ignore-system-fonts which keeps default to false. For a configuration item, it is not necessary to have ignoreSystemFonts since we will document the default value for the configuration at the same time. For example, To use system fonts, the configuration will be ignoreSystemFonts=false. This is harder to interpret.

7sDream commented 1 month ago

Make sense. Done.