nvf icon indicating copy to clipboard operation
nvf copied to clipboard

Added Tex Language Support

Open isaacST08 opened this issue 1 year ago • 33 comments

Added support for the tex language. Added the texlab LSP along with config options for build/compiling:

  • Compiling package option
  • Compiling executable option
  • Compiling executable args option
  • PDF preview using your PDF viewer of choice
  • PDF preview update on save option

Not implemented (yet):

  • Support for other LSPs
  • Formatting support via nvf options
  • Texlab options (There is however an extraLuaSettings option for texlab that allows setting these options in the meantime):
    • chktex
    • diagnosticsDelay
    • diagnostics
    • symbols
    • formatterLineLength
    • bibtexFormatter
    • latexFormatter
    • latexindent
    • completion
    • inlayHints
    • experimental

Defaults mostly follow the texlab defaults with some exceptions such as the default compiler being tectonic and the default file viewer being okular.

(I also alphabetically sorted the language imports in the modules/plugins/languages/default.nix)

isaacST08 avatar Jan 19 '25 04:01 isaacST08

This should satisfy #525

isaacST08 avatar Jan 19 '25 04:01 isaacST08

:rocket: Live preview deployed from 4b95ae106c832bea347ad2bd53f2c40d880f0d27

View it here:

Debug Information

Triggered by: isaacST08

HEAD at: feature-language-tex

Reruns: 1733

github-actions[bot] avatar Jan 19 '25 18:01 github-actions[bot]

Feature essentially done.

Implemented:

  • Lsp
    • Texlab (Recommended)
      • All features currently provided as documented by texlab
  • Builders/Compilers
    • Tectonic (Recommended)
    • latexmk
  • PDF Viewers
    • okular
    • zathura
    • sioyek
    • qpdfview
  • Forward search
  • Formatter

Not implemented:

  • Inverse search
    • Not planned as widely different between pdf viewers and configuring through nix might not even be possible for some (at least not easily) (okular...).
    • Sioyek does have inverse search support via cli args and (should) be working properly with default values.

Due to the scale that this language implementation ended up being I organized it differently compared to the other languages; it is in multiple files and modules, all called by a default.nix in a tex directory. The implementation of the second largest language is more that 5 times smaller and I felt like a single 1500 line file wasn't ideal and the current implementation was a more appropriate approach.

I have not tested every single combination of options as that would take forever. I have tested many of the more relevant ones but I have not tested the texlab experimental settings for instance.

The config I am currently using is as follows:

settings.vim.languages.tex = {
  enable = true;
  lsp.texlab = {
    forwardSearch.enable = true;
  };
  build = {
    builders.tectonic.enable = true;
    forwardSearchAfter = true;
    onSave = true;
  };
  pdfViewer.zathura.enable = true;
  extraOpts.texFlavor = {
    enable = true;
    flavor = "tex";
  };
};

I have created frameworks for easily adding both new pdf viewers and new tex builders/compilers. Adding other LSP servers should be not too complicated if texlab proves to be fundamentally inadequate.

Overall, this implementation will cover all of my needs for tex and includes many other features I will likely not use but have been implemented to achieve near feature parity with many of the implemented programs which will hopefully cover most of what anyone else requires. If not, in a few places I added methods to be able to add custom configuration just in case.

There are no other features I plan to add for the time being, but I will try to fix bugs as they come up on my side.

isaacST08 avatar Jan 31 '25 02:01 isaacST08

Looks good from a first glance. Thank you for your hard work.

I'm too tired to review in full at the moment, but I'll properly review tomorrow when I wake up. One thing that comes to mid is that mkEnableTreesitterOption could probably be put into the extended library, so that other modules can use it as well.

NotAShelf avatar Jan 31 '25 12:01 NotAShelf

If you could also fix the Cı errors by then, that'd be great.

NotAShelf avatar Jan 31 '25 12:01 NotAShelf

Sounds good, I’ll get onto those things when I next get a moment

isaacST08 avatar Feb 02 '25 07:02 isaacST08

I made some of the changes you requested.

I made a mkEnableTreesitterOption function in the extended library. I must say I'm not too familiar with extending lib so it's a little different from how the original function was. I didn't want to start messing with the imports of the existing lib extensions by finding a way to add config as an input so the function itself requires passing the config to it as one of its inputs. If there's a better/cleaner way to do this feel free to make those changes or provide a source where I can learn how to do it properly.

If there are any other changes you wish to be made don't hesitate to let me know.

On an aside, I have been testing this PR this whole time for academic work and I can now also say that as I have now started using bibtex I can say that it has been working flawlessly and multi-file build and re-building has been working great too.

isaacST08 avatar Feb 12 '25 00:02 isaacST08

I'm going to have to ask you to rebase.

With #605 we have transitioned to an alternative method that is faster and cheaper than flake inputs for plugins, which does meant that there will be merge conflicts I will need you to fix before I can merge this.

Meanwhile, I'll provide a complete review, there shouldn't be too much to cover this time around.

NotAShelf avatar Feb 12 '25 00:02 NotAShelf

I was wrong, that is a lot of changes. Although, I do appreciate your good work. TeX ecosystem is incredibly large and I was partially intimidated from working on TeX support myself.

NotAShelf avatar Feb 12 '25 01:02 NotAShelf

I've done a rebase but the new-file-templates.nvim plugin is going to be an issue again for generating new tex files. Can you give me a rundown of how one might handle changing the source to the fork I made fixing the issue until the fix gets merged into the main repo?

In the meantime, I'll start working on the fixes you requested.

Just as a note, after rebasing by config became broken. The error was a big mess of nvim pluggin names from the looks of it (I can post the stacktrace error somewhere if you'd like but I don't know if here is where you want it). Removing settings.vim.lsp.lspsaga.enable fixed the issue. I don't know if this expected behaviour and that option is now (or will be) deprecated or if this is something to look into.

isaacST08 avatar Feb 12 '25 01:02 isaacST08

LSPSaga is addressed in #629, alongside many other plugins. I'll merge it somehow tomorrow if my config decides to build.

Can you give me a rundown of how one might handle changing the source to the fork I made fixing the issue until the fix gets merged into the main repo?

npins add --name <name> github <owner> <repo> shoudl work, it's a little less declarative than a flake input but same idea.

NotAShelf avatar Feb 12 '25 02:02 NotAShelf

@NotAShelf is there something I'm missing for this to be merged? Are there more changes you have for me? Or are you still busy with the migration and this should wait until later?

isaacST08 avatar Mar 03 '25 21:03 isaacST08

Sorry, I've put this aside for a while because we were trying to decide how language modules would be handled in the future. I'd like to get rid of lspconfig once Neovim 0.11 drops, but I am not exactly sure if I want to do so yet. We already started working on #593, but it's stale because there isn't an agreement on the schema.

This is a relatively large PR, and I quite unsure whether I will be able to handle the module myself if I merge it as we modify the language module. I'll revisit this PR in the coming days (before this weekend) if I cannot decide on a way to go for the language modules as a whole.

NotAShelf avatar Mar 03 '25 22:03 NotAShelf

@isaacST08 any chance you can make your fork usable by a nix flake? I'm just coming to nix and so I couldn't figure out how to use an alternate github. LaTeX is essential to my workflow, so the lack of support by nvf has been problematic to me.

ndrwstn avatar Mar 06 '25 16:03 ndrwstn

isaacST08 any chance you can make your fork usable by a nix flake? I'm just coming to nix and so I couldn't figure out how to use an alternate github. LaTeX is essential to my workflow, so the lack of support by nvf has been problematic to me.

You should be able to use a branch on someone's fork by replacing the url for nvf with github:isaacST08/nvf/feature-language-tex.

alfarelcynthesis avatar Mar 06 '25 16:03 alfarelcynthesis

Sorry, I've put this aside for a while because we were trying to decide how language modules would be handled in the future. I'd like to get rid of lspconfig once Neovim 0.11 drops, but I am not exactly sure if I want to do so yet. We already started working on #593, but it's stale because there isn't an agreement on the schema.

This is a relatively large PR, and I quite unsure whether I will be able to handle the module myself if I merge it as we modify the language module. I'll revisit this PR in the coming days (before this weekend) if I cannot decide on a way to go for the language modules as a whole.

Following up on this, we have mostly agreed on a schema. I'm not sure how feasible its implementation will be for Tex (although I would like to take a look after I deliver my due work). I'm going to review this PR one more time tomorrow, if I forget please remind me. We can probably merge this as is I've covered most, if not all little nitpicks I could find.

NotAShelf avatar Mar 06 '25 20:03 NotAShelf

There seems to be a few typos caught by the CI and an evaluation failure. Please address those before I can merge.

NotAShelf avatar Mar 06 '25 20:03 NotAShelf

There seems to be a few typos caught by the CI and an evaluation failure. Please address those before I can merge.

I have fixed these but have done so via online GitHub so I will update on my system later today.

I’m a little baffled as to why I was able to build successfully with the evaluation error on my machine but regardless it should be fixed now. I will still test properly when I get back to my machine though.

isaacST08 avatar Mar 07 '25 19:03 isaacST08

isaacST08 any chance you can make your fork usable by a nix flake? I'm just coming to nix and so I couldn't figure out how to use an alternate github. LaTeX is essential to my workflow, so the lack of support by nvf has been problematic to me.

You should be able to use a branch on someone's fork by replacing the url for nvf with github:isaacST08/nvf/feature-language-tex.

Yes this is how you would use my fork. I will keep updating my fork with any changes that come to nvf main every few days until it gets merged.

Additionally, if you run into any problems please open an issue on my fork’s GitHub page. This can either be for bugs or questions with configuration as I don’t know how to get you a branched version of the nvf appendix B docs.

isaacST08 avatar Mar 07 '25 19:03 isaacST08

I'm guessing somehow lazy eval skipped the faulty parts. Please feel free to ping me once you apply your changes, and I'll work on merging this. If there's any more nits, I can probably fix them myself over the weekend.

NotAShelf avatar Mar 07 '25 20:03 NotAShelf

@NotAShelf I've applied the changes and it works on my machine (famous last words lol). Sorry for the delay, I had quite a big assignment due (in latex funnily enough). But at least I can say once again that its been pretty roughly tested and I haven't run into any bugs for a while (at least within the realm of my use cases).

isaacST08 avatar Mar 08 '25 07:03 isaacST08

No worries, I was caught up in meetings all day and only now found the time to check my notifications. I'll be home in roughly an hour, then I'll look at this PR again. I'd think everything is done, the CI was passing last time and if there are any other nits, I'll get them myself as I've said before.

NotAShelf avatar Mar 08 '25 16:03 NotAShelf

There are some stylistic choices that I don't particularly agree with here, but I think I've nitted enough. None of them are critical, and I'll get them whenever I refactor the language module myself.

One thing of note:

config = mkIf (cfg.enable && (any (x: x.enable) (attrValues cfg.lsp))) {
    vim.lsp.lspconfig.enable = true; # Enable lspconfig when any of the lsps are enabled
  };

This is defined explicitly in modules/plugins/languages/tex/lsp/default.nix but I'd argue that it should be a part of the lspconfig module. Is there a specific reason this is in languages/tex?

Other than that, there's nothing else left. Thank you for your efforts. If you could just get the CI failure (there's an infrec from the module) I'll merge this.

I think that I wrote that quite early on in the project. I was still learning the code base and I wanted to stay in my own lane and modify as few other files as possible. I think it could belong in either location. However, I think putting it in the lspconfig module could make it a little less concise in terms of calling options of other modules but the exact same thing can be said for the other direction. It's your code base so inevitably it's your decision.

Again, I don't know why the CI is catching these errors but why I build it is not. I'm using the pdfViewer config options in my set up so it is getting evaluated but I don't see why the CI is getting an infrec but I'm not. I made a tweak that might fix it but I'm going to have to wait for a different day when I'm more awake to properly investigate an infrec, especially one that I apparently will have to solve using purely theory alone. If you can see a quick fix for it that I'm missing please feel free to make that change.

isaacST08 avatar Mar 10 '25 16:03 isaacST08

Additionally, if you run into any problems please open an issue on my fork’s GitHub page. This can either be for bugs or questions with configuration as I don’t know how to get you a branched version of the nvf appendix B docs.

No worries; I poked around in the code until I got it to work. I did have a couple questions regarding: use and a possible bug I wanted to bring to your attention--but I don't think you have issues open on your fork? I'll save the conversation for there rather than here though.

ndrwstn avatar Mar 20 '25 19:03 ndrwstn

orries; I poked around in the code until I got it to work. I did have a couple questions regarding: u

Sorry, I forgot to turn issues on, they are on now though.

isaacST08 avatar Mar 20 '25 19:03 isaacST08

@NotAShelf Apologies for the delay but I spent some time working on some other projects and university. Though in that time I learned more about nix and refactored the pdf viewer section to fix the infrec error accordingly (getting adequate sleep now that finals are done helped too). Additionally, I figured out how to run the checks on my own system (I think) which now evaluates without errors which I'm hoping will be the same for when the CI runs its own checks.

Once again, it is ready for your review and as always let me know if anything goes awry and I'll do my best to fix it.

isaacST08 avatar May 04 '25 01:05 isaacST08

No worries, I believe some people have just elected to use your fork instead for Tex support.

We have finally completed the first step of the LSP refactor with the release of Neovim 0.11. This is a much simpler API, and uses Neovim directly to manage language servers instead of relying on lspconfig. One problem for now is that this API is exclusive to > 0.11, and causes breakage for stable channel (which still ships 0.10, so the language modules have not been migrated yet; we are waiting for 25.05 release. If you could update the PR to use the new API (which is quite simple, and should not require significant refactor) then I can just merge this PR with NixOS 25.05 release.

NotAShelf avatar May 04 '25 10:05 NotAShelf

No worries, I believe some people have just elected to use your fork instead for Tex support.

We have finally completed the first step of the LSP refactor with the release of Neovim 0.11. This is a much simpler API, and uses Neovim directly to manage language servers instead of relying on lspconfig. One problem for now is that this API is exclusive to > 0.11, and causes breakage for stable channel (which still ships 0.10, so the language modules have not been migrated yet; we are waiting for 25.05 release. If you could update the PR to use the new API (which is quite simple, and should not require significant refactor) then I can just merge this PR with NixOS 25.05 release.

I will do this switch when we get closer to the NixOS 25.05 release. I think a mini manual outlining how one might go about switching to the new api and guidelines on what you would like to see would be very helpful. If something like this already exists please link it; I took a brief search but couldn't find anything.

Since I know that at least one person is using my fork to gain TeX support I will leave it using the current implementation until Neovim 0.11 becomes part of the stable channel.

isaacST08 avatar May 04 '25 18:05 isaacST08

There is no guideline yet, but I'll add it once I sit down to revise the documentation. We're not in a rush anyway.

NotAShelf avatar May 04 '25 19:05 NotAShelf

I don't wanna bother you but is this still being developed/considered for merge? I'm gonna start university this year and I'd like to take notes with latex so tex support would be really useful

ItsLiyua avatar Jul 03 '25 14:07 ItsLiyua