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

languages/nix: add nixd to supported LSP servers #458

Open
wants to merge 3 commits into
base: v0.7
Choose a base branch
from

Conversation

Elias-Ainsworth
Copy link

@Elias-Ainsworth Elias-Ainsworth commented Nov 11, 2024

Add nixd as a possible lsp for nix.

Sanity Checking

  • I have updated the changelog as per my changes.
  • I have tested, and self-reviewed my code.
  • Style and consistency
    • I ran Alejandra to format my code (nix fmt).
    • My code conforms to the editorconfig configuration of the project.
    • My changes are consistent with the rest of the codebase.
  • If new changes are particularly complex:
    • My code includes comments in particularly complex areas
    • I have added a section in the manual.
    • (For breaking changes) I have included a migration guide.
  • Package(s) built:
    • .#nix (default package)
    • .#maximal
    • .#docs-html
  • Tested on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin

Add a 👍 reaction to pull requests you find important.

Add nixd to flake.nix and legacyPackages.nix.

Format with alejandra.
@NotAShelf NotAShelf changed the title Add nixd. languages/nix: add nixd to supported LSP servers Nov 11, 2024
Copy link
Owner

@NotAShelf NotAShelf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM.

Left a few comments for consistency, if you could also add an entry to the changelogs this should be good to go.

flake.nix Outdated Show resolved Hide resolved
flake.nix Show resolved Hide resolved
modules/plugins/languages/nix.nix Show resolved Hide resolved
@NotAShelf
Copy link
Owner

Almost forgot, please rebase on the v0.7 branch as main is currently supposed to be in a feature freeze.

@diniamo
Copy link
Collaborator

diniamo commented Nov 11, 2024

There is a reaoson we didn't already have nixd, so we should be a little more cautious about merging this. I think some features rely on LSP settings (which the user can't configure until the rewrite).

@Elias-Ainsworth
Copy link
Author

I submitted a pr for the rebased commit. Should I close this one considering it may be unecessary?

@diniamo
Copy link
Collaborator

diniamo commented Nov 11, 2024

You could've just pushed that to this branch, and set the target branch to v0.7 (which I would still prefer to preserve comments).

@Elias-Ainsworth Elias-Ainsworth changed the base branch from main to v0.7 November 12, 2024 20:51
@Elias-Ainsworth
Copy link
Author

You could've just pushed that to this branch, and set the target branch to v0.7 (which I would still prefer to preserve comments).

Sorry...I'm a little stupid

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 this pull request may close these issues.

3 participants