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

showExportFileIn: systemDefault does not work for non-ASCII file paths #837

Closed
YDX-2147483647 opened this issue Nov 17, 2024 · 4 comments · Fixed by #838
Closed

showExportFileIn: systemDefault does not work for non-ASCII file paths #837

YDX-2147483647 opened this issue Nov 17, 2024 · 4 comments · Fixed by #838

Comments

@YDX-2147483647
Copy link

YDX-2147483647 commented Nov 17, 2024

Describe the bug

  1. Configure tinymist.showExportFileIn to systemDefault.
  2. Open any *.typ under a non-ASCII path.
  3. Click the Show exported PDF button. Nothing happens.

Package/Software version:

VSCode version:

Version: 1.95.3 (system setup)
Commit: f1a4fb101478ce6ec82fe9627c43efbf9e98c813
Date: 2024-11-13T14:50:04.152Z
Electron: 32.2.1
ElectronBuildId: 10427718
Chromium: 128.0.6613.186
Node.js: 20.18.0
V8: 12.8.374.38-electron.0
OS: Windows_NT x64 10.0.19045

tinymist extension version: v0.12.2.

Logs:

tinymist server log(Output Panel -> tinymist):

[2024-11-17T10:33:30Z INFO  sync_lsp] handling workspace/executeCommand - (9004) at Instant { t: 18666.22s }
[2024-11-17T10:33:30Z INFO  sync_lsp] handling workspace/executeCommand - (9005) at Instant { t: 18666.22s }
[2024-11-17T10:33:30Z INFO  tinymist::task::export] RenderActor(Pdf { creation_timestamp: None }): exporting EntryState { rooted: true, root: Some("d:\\…"), main: Some(\main.typ) } to "d:\\…\\main.pdf"
[2024-11-17T10:33:30Z INFO  tinymist::task::export] RenderActor(Pdf { creation_timestamp: None }): exporting EntryState { rooted: true, root: Some("d:\\…"), main: Some(\main.typ) } to "d:\\…\\main.pdf"
[2024-11-17T10:33:30Z INFO  tinymist::task::export] RenderActor(Pdf { creation_timestamp: None }): export complete
[2024-11-17T10:33:30Z INFO  tinymist::actor::typ_client] CompileActor: on export end: "d:\\…\\main.typ" as Some("d:\\…\\main.pdf")
[2024-11-17T10:33:30Z INFO  sync_lsp] handled  workspace/executeCommand - (9004) in 12.84ms
[2024-11-17T10:33:30Z INFO  tinymist::task::export] RenderActor(Pdf { creation_timestamp: None }): export complete
[2024-11-17T10:33:30Z INFO  tinymist::actor::typ_client] CompileActor: on export end: "d:\\…\\main.typ" as Some("d:\\…\\main.pdf")
[2024-11-17T10:33:30Z INFO  sync_lsp] handled  workspace/executeCommand - (9005) in 12.31ms

The folder path, which I truncated to , contains Han characters.

tinymist client log(Help -> Toggle Developer Tools -> Console):

ERR [uncaught exception in main]: Failed to open: 系统找不到指定的文件。 (0x2): Error: Failed to open: 系统找不到指定的文件。 (0x2)

系统找不到指定的文件 means the system cannot find the specified file.

Additional context

The feature was added in #636.

The VS Code API vscode.env.openExternal may not support non-ASCII file paths.
See microsoft/vscode#88273 or microsoft/vscode#85930.

Currently, opening with the system default robustly might be too complicated to implement or maintain. Therefore, I suggest documenting the edge case and/or recommending third-party extensions (as has been done with vscode-pdf).

Reference implementations

LaTeX Workshop

They implement latex-workshop.view.pdf.external.viewer.command by spawning a new program.

Pseudo code extracted from https://github.com/James-Yu/LaTeX-Workshop/blob/fb0e2b0c43fcdc1e888f036a2da1c19070f8d3a3/src/preview/viewer.ts#L218:

function viewInExternal(pdfUri: vscode.Uri): void {
    const configuration = vscode.workspace.getConfiguration('latex-workshop')
    let command = configuration.get('view.pdf.external.viewer.command') as string
    let args = configuration.get('view.pdf.external.viewer.args') as string[]
    args = args.map(arg => arg.replace('%PDF%', pdfUri.fsPath))
    const proc = cs.spawn(command, args, {cwd: path.dirname(pdfUri.fsPath), detached: true})
}

By the way, they use vscode.env.openExternal to viewInBrowser
https://github.com/James-Yu/LaTeX-Workshop/blob/fb0e2b0c43fcdc1e888f036a2da1c19070f8d3a3/src/preview/viewer.ts#L116

A dedicated extension

Disclamer: I am not actively using the extension. I am astonished when I see my own comments in tjx666/open-in-external-app#5.

There are two ways to open file in external applications.

https://github.com/tjx666/open-in-external-app/blob/bbf107674fbc053e52b61efb69023e2719ce6438/README.md?plain=1#L90-L98

@Myriad-Dreamin
Copy link
Owner

Myriad-Dreamin commented Nov 17, 2024

Let's turn to use rust's open crate. Unluckily, we will run into same bad cases had in npm's open package.

Node package: sindresorhus/open

This package has one limit that can't open a file which is also made by electron.

Edit: The following code works perfectly with "Adobe Acrobat", "Visual Studio Code", and Edge as default app:

#[cfg(not(target_os = "windows"))]
let do_open = ::open::that_detached;
#[cfg(target_os = "windows")]
fn do_open(path: impl AsRef<std::ffi::OsStr>) -> std::io::Result<()> {
    ::open::with_detached(path, "explorer")
}

@YDX-2147483647
Copy link
Author

Thanks for such a quick fix, and looking forward to the next release!

@Myriad-Dreamin
Copy link
Owner

@YDX-2147483647 you could check https://github.com/Myriad-Dreamin/tinymist?tab=readme-ov-file#installing-regularnightly-prebuilds-from-github to install latest prebuilts quickly

@YDX-2147483647
Copy link
Author

Thanks. I can verify that Sumatra PDF also works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants