astro/deadnix

JSON as an alternative output format

ilkecan opened this issue · 11 comments

ilkecan commented

Hi. How would you feel about adding an alternative output format that is easier to parse (like JSON)? I am interested in this to integrate deadnix into my editor using null-ls.vim. Would you accept such a PR?

astro commented

Fine by me

water-sucks commented

Just wondering, why is this an optional feature not enabled by default unless you compile from source manually? I would like to add it to my NixOS configuration to use with null-ls but the version in nixpkgs is 0.1.3 (without the JSON feature needed), and I attempted to use the flake but the feature flag was not enabled by default.

I'd like to know if there is a way to add this feature flag by overriding naersk-lib.buildPackage somehow in my own configuration flake (I've searched and haven't found any solutions till now) or if it should be enabled by default (I can make it a PR).

ilkecan commented

Since this feature is not required for the core functionality and needs additional dependencies, I disabled it by default in the PR.

If it helps, this is how I am currently using it:

  deadnix = unstable.deadnix.overrideAttrs(oldAttrs: rec {
    src = pkgs.fetchFromGitHub {
      owner = "astro";
      repo = "deadnix";
      rev = "0f78b370f0aab6f9512fa243e7a9da595b44f63b";
      sha256 = "sha256-TCGbFi0VbooyBv9vyOt1qtBFmTsQXH8sPOdMh9agMGU=";
    };
    cargoBuildFeatures = oldAttrs.cargoBuildFeatures  ++ [ "json-out" ];
    cargoDeps = oldAttrs.cargoDeps.overrideAttrs (_oldAttrs: {
      inherit src;
      outputHash = "sha256-A3i8YfmoFkcI/uUbaOIv4oCqnIqIiZtMH0ksOE2SSmk=";
    });
  });
astro commented

nixpkgs needs a deadnix version bump anyway. Should we enable the feature by default when using deadnix from nixpkgs?

water-sucks commented

Even if deadnix itself isn't exactly a language server, I think it's a good idea to add it to the default features list so people who want to use it as an LSP source can do it without using a overlay.

I used the code and it worked, but it just seems a little inconvenient because null-ls spits out a weird error if the JSON output isn't enabled and people don't know where to look.

ilkecan commented

To be clear, I would also prefer it being enabled by default in Nixpkgs. What I meant was, I didn't want to change the default behaviour myself. I think that decision should be made by Astro.

astro commented

@ilkecan I don't have a strong opinion on this myself, so let's enable the feature by default.

@water-sucks I'd love to add LSP functionality one day. On the other hand, editors talk to one LSP server only, right? Then it would make sense to integrate this with rnix-lsp.

ilkecan commented

The built-in Neovim LSP client can talk with multiple servers at the same time and this seems to also be the case for some other clients:

water-sucks commented

Depends on the editor in question. In Neovim you definitely can attach multiple language servers to a buffer; that's the way null-ls works in tandem with rnix and whatever other language server you configure.

I can try adding LSP capability to deadnix myself because it already has rnix as a dependency, though I'm not an expert in Rust so it could take a bit of time for me to do.

  • 👍1
Artturin commented

it would be good if deadnix was always compiled with this..

astro commented

@Artturin That is the case since 9617e4c I have released 0.1.5 which is merged in nixpkgs.

  • 👍1