Add support for launching preview via lsp

[wisp3rwind]

This add supports for starting preview via the language server instead of running a separate preview process and connecting to it via websocat. In particular, this means that an existing tinymist installation (e.g. system-wide or via mason) and its configuration via the regular language server configuration can be leveraged and there's no need to download executables. I'd hope that it also helps to save resources (RAM+CPU) due to running only a single process but I'm not sure whether that's actually true (i.e. whether the language server and preview task actually share resources).

Essentially, I had a look at the VSCode plugin, and replicated what it does. This turned out to be pleasantly simple given nvim's lsp infrastructure.

This is still a work-in-progress, I'd surprised if there were no regressions. I also left a bunch of FIXME comments that should probably be addressed. Documentation also needs updating.

I tried to keep support for the previous installation method: Unless the use_lsp = true option is given, nothing user-visible should have changed. I did refactor quite a bit of the code, such as

• merge server/init.lua and server/manager.lua: the distincation was a bit vague anyway, and given how event handling changed, I was running into circular imports otherwise
• servers are now stored in a sequence, and finding a specific on (e.g. by path) is done with a linear search. The reason is that this is much simpler since I need to retrieve servers by task_id as well. Given that there should be at most a handful of entries, performance is unlikely to be an issue.
• event handling/listener registration is changed a bit, since not all events received from the lsp side are clearly attributable to a single preview task. That's also not necessary anyway, since the actual listener doesn't depend on the specific task anyway (slight caveat here for the suppress handling, which I disabled for now, see the FIXME. That can probably be handled at a lower level, though.)

In its current state, this code works for me. Thus, I'd be happy about some initial feedback:

• Is this in scope for the plugin, could it be merged once remaining issues are addressed? I've only now seen Tinymist preview feature not yet released #47 (@max397574 this PR might be interesting to you?). If and when this is ready to go, I can also try to split the PR into smaller, easier-to-review commits.
• How would you imagine the longer-term future of this plugin: Should support for the old and new preview methods be kept for the time being? Should this be converted to support just the lsp method? (Which should allow removing a lot of code, such as executable fetching.)
• I'm not at all familiar with lua, just learning it a little as I modify the code. Any comments on the general code structure, or how you would envision it?

[wisp3rwind]

Just noticed that this can be simplified further: tinymist sends window/showDocument requests to jump to the source, and nvim already handles that. Thus, the custom jumping logic can be removed. See Myriad-Dreamin/tinymist#1450

[wisp3rwind]

Just pushed 2 additional commits: Now, the 3 commits in this PR are:

1. I described above
2. remove all code related to fetching binaries and starting them, only support LSP
3. add a lazy.lua spec to automatically lazy-load the plugin for Lazy users

I'd argue that it might be the simplest way forward (if you think that these changes are generally appropriate for the plugin) to just go all the way and remove the old way of fetching binaries, and bump the version to 2.0.0, instead of shipping all the code to support both simultaneously.

EDIT: Note that as it is, this PR requires the latest tinymist (0.13.10) and probably also a recent neovim (not sure which version, however: I locally tested this on 0.11.0).

[ReKylee]

ngl, can't wait for this to get pulled already haha

[bluss]

Nice work. Is this usable as a fork for now?

[wisp3rwind]

Nice work. Is this usable as a fork for now?

It should just work. I've been using it for the last few weeks (admittedly not much) without issues. There might of course be sharp edges and definitely missing documentation.

EDIT: Would be happy about any feedback, in particular regarding any issues this branch might cause.

[bluss]

I changed to your fork and branch and did not change my configuration, and the change resulted in:

Error executing vim.schedule lua callback: /usr/share/nvim/runtime/lua/vim/_system.lua:251: ENOENT: no such file or directory (cmd): 'firefox %s -P typst-preview --class typst-preview'

The existing configuration open_cmd = 'firefox %s -P typst-preview --class typst-preview' does not work with this branch. I think it's probably clear why in the code, just wanted to report this fact.

Edit: opts.cmd = {config.opts.open_cmd } ensures that opts.cmd is always a table, so I cant configure a command with multiple arguments (new config would be open_cmd = { "firefox", "-P", "typst-preview" },) but this doesn't work with the current implementation.

Not saying this is the way to go, but at least it can work, with new configuration, this way. But morally you probably need to continue to respect the old configuration or document how to change configuration.

diff --git lua/typst-preview/utils.lua lua/typst-preview/utils.lua
index 923641b..d19231f 100644
--- lua/typst-preview/utils.lua
+++ lua/typst-preview/utils.lua
@@ -6,7 +6,11 @@ local M = {}
 function M.visit(link)
   local opts = {}
   if config.opts.open_cmd ~= nil then
-    opts.cmd = {config.opts.open_cmd }
+    if type(config.opts.open_cmd) == "table" then
+      opts.cmd = config.opts.open_cmd
+    else
+      opts.cmd = {config.opts.open_cmd }
+    end
   end
   vim.ui.open('http://' .. link, opts)
 end

With that it "works", but :TypstPreview becomes more unreliable - w.r.t for example open preview twice in the same neovim, once each for two different buffers, I don't get that to work (it just opens the first preview again). Closing preview for a buffer and opening it with :TypstPreview again (opens to a browser window that doesn't connect to a live preview). So there are certainly challenges.

💥 QA bomb 😉

[wisp3rwind]

The existing configuration open_cmd = 'firefox %s -P typst-preview --class typst-preview' does not work with this branch. I think it's probably clear why in the code, just wanted to report this fact.

Good call: I refactored the case where no open_cmd is given and the default system opener should be used, and unintentionally also changed how the open_cmd config value is used. I just pushed a commit that should restore the old behaviour.

With that it "works", but :TypstPreview becomes more unreliable - w.r.t for example open preview twice in the same neovim, once each for two different buffers, I don't get that to work (it just opens the first preview again). Closing preview for a buffer and opening it with :TypstPreview again (opens to a browser window that doesn't connect to a live preview). So there are certainly challenges.

That's unexpected, opening previews for different buffers in a single nvim instance should definitely work (and it does for me). I don't really have an idea what could be going on here. (Unless that's actually due to the incorrect open_cmd handling, I think your workaround might still leak the %s unformatted to the command, not sure what the result of that would be.)

Could you enable debug logging for the plugin via

    {
      'chomosuke/typst-preview.nvim',
      opts = {
        open_cmd = "firefox %s ...",
        debug = true,
      },
    },

and provide the log from ~/.local/share/nvim/typst-preview/log.txt (default on Linux; otherwise should be in the directory given by :echo stdpath("data")) after using preview in a way that exhibits the issue? Maybe, that could give some hints on what's going on. (The log contains paths to your documents, you might want to check whether there's anything you'd like to redact.)

Rough edges that I'm aware of are

• When starting preview very quickly after launching nvim, before the LSP is up, opening the preview will fail. It might be possible to handle this more gracefully (produce a better error message, detect whether the LSP is starting and if so start preview once it's ready.)

• It appears that tinymist suspends the preview after some time of inactivity, which will make it lose interactivity. Reloading the preview (i.e. reloading the page in Firefox) will restore interactivity in that case. @Myriad-Dreamin is that a correct assessment of what's going on? Is there some way from the editor side to detect this, and to keep preview alive/restart it once there's activity in the editor again?

It doesn't really sound like those are your issue, however. The second point might contribute to why you perceive the plugin as less reliable than before?

💥 QA bomb 😉

😆 I was afraid it might be worse. Luckily, this actually simplifies the code in some ways.

[bluss]

It appears that tinymist suspends the preview after some time of inactivity, which will make it lose interactivity. Reloading the preview (i.e. reloading the page in Firefox) will restore interactivity in that case.

This is not new - unless the time until this happens has changed. typst-preview.nvim's preview generally behaves this way.

[bluss]

So far so good! The new update works with unchanged config and it does support multiple previews at the same time, I guess for unknown reason why. (The %s you mentioned could not have been an issue as there was no %s in the command I used for the workaround.) I'll continue to use and see how it goes..

[Myriad-Dreamin]

It appears that tinymist suspends the preview after some time of inactivity, which will make it lose interactivity. Reloading the preview (i.e. reloading the page in Firefox) will restore interactivity in that case. @Myriad-Dreamin is that a correct assessment of what's going on? Is there some way from the editor side to detect this, and to keep preview alive/restart it once there's activity in the editor again?

@wisp3rwind @bluss To avoid resource leaking we made a timer to cleanup inactive preview instances. Perhaps we can improve it by some heartbeats from the webpage or the typst-preview.nvim, but introduce the risks to leak resource.

Random guess: tinymist won't remove a preview instance if there is any connection to a webpage, but firefox/chrome will freeze a inactive webpage after a while to reduce energy consumption, which close the only active connection to the preview instance.

[bluss]

I don't know if I can "report bugs" vs this branch here. I also don't know (right now) if this bug is specific to this branch. Anyway, here's the scenario:

1. Open a .typ document in nvim
2. Launch preview (using this PR's branch), showing preview in Firefox
3. Double click in preview to focus location in source text in nvim. This works fine
4. Find a place to double click that focuses a different file - for example a source file from a package. So some content is produced (or indirectly produced) by a package. Nvim opens that file and focuses that source file from the package
5. (Optional step) click something else again in preview that focuses a location in your original own document
6. Close the extra file that was opened in step (4)
7. The preview process now shuts down (because file (4) was closed?). This can be confirmed by reloading in the browser window (the process is dead)

It's unexpected that the preview shuts down just because an auxiliary buffer is closed. The main document is still open and the user still wants to continue editing the main document.

Using pinMain or not has no effect on this.

I have been using this branch since the last message (a few weeks), but I'm going back to the original preview. Two reasons:

• The above issue
• Issuing :e to re-read the current file (happens often by habit) also kills the preview when using this branch.

If they can be fixed, then I don't know any other regressions.

[Myriad-Dreamin]

@bluss I recently wrote some integration tests for neovim, like pdf preview will work good when exportPdf on "never", "onSave" and "onType". Making similar tests for websocket preview may really ensure reliable behaviors in future.

[bluss]

That's nice. I think the above stuff are something higher level, it's probably registered for some event and closing down a little too eagerly. I think, at least. Or, less likely?, there's something with the separate process that's more durable anyway.

[Myriad-Dreamin]

That's nice. I think the above stuff are something higher level, it's probably registered for some event and closing down a little too eagerly. I think, at least. Or, less likely?, there's something with the separate process that's more durable anyway.

@bluss No, it is not testing some low-level internal things, but to wait and judge final PDF changes with some user actions for testing PDF preview. Some cheap event callbacks are made, which is more reliable than polling fs to wait PDF changes. Similarly, we can test typst-preview by sending user actions, or injecting or simulating rare input like system hibernation.

[wisp3rwind]

I don't know if I can "report bugs" vs this branch here. I also don't know (right now) if this bug is specific to this branch. Anyway, here's the scenario:

Sure 👍 I'm not using Typst as much right now, but I still want this to be robust, so happy about any feedback.

I can reproduce using your steps (5. not required as you note).

Looks like a bug in the nvim plugin where we shutdown previews too eagerly when a language server detaches from a buffer. (I think what the code was intended to do is remove stale preview tasks when the server shuts down, but I got it quite wrong.)

I'll look into fixing this later.

Issuing :e to re-read the current file (happens often by habit) also kills the preview when using this branch.

Need to check, but could be the same issue.

[bluss]

@bluss No, it is not testing some low-level internal things, but to wait and judge final PDF changes with some user actions for testing PDF preview. Some cheap event callbacks are made, which is more reliable than polling fs to wait PDF changes. Similarly, we can test typst-preview by sending user actions, or injecting or simulating rare input like system hibernation.

Maybe a misunderstanding. I was just guessing that the bugs I reported here are some things that might be due to "it's probably registered for some event and closing down a little too eagerly."

[bluss]

While I know this PR has benefits, I prefer the current model (without this PR) because the preview is more “durable” for example continuing unbroken during buffer reloads (:e<CR>). I wonder if we can find a best of both worlds somehow.