Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

nix: format shell.nix #51343

Closed
wants to merge 7 commits into from
Closed

nix: format shell.nix #51343

wants to merge 7 commits into from

Conversation

keegancsmith
Copy link
Member

I don't know which formatter this is running, but it is what happens when I run M-x nix-format-buffer in emacs.

Test Plan: nix develop

I don't know which formatter this is running, but it is what happens
when I run M-x nix-format-buffer in emacs.

Test Plan: nix develop
@keegancsmith keegancsmith requested a review from Strum355 May 2, 2023 10:35
@cla-bot cla-bot bot added the cla-signed label May 2, 2023
@Strum355 Strum355 added the nix For Nix(OS) related issues/PRs label May 2, 2023
@Strum355
Copy link
Contributor

Strum355 commented May 2, 2023

This seems to be using nixfmt instead of nixpkgs-fmt. Do you have any particular preference in the output? I lean towards nixpkgs-fmt personally (the line breaks feel a bit awkward imo)

We could add a formatter thatd be invoked consistently by nix fmt that I just discovered

@keegancsmith
Copy link
Member Author

pushed a commit to use nixpkg-fmt instead. Also I wasted like 15min trying to get format working cleanly in flake.nix (ie with a map over default systems), and I ended up polishing a turd. So I just ignored that and manually ran nixpkg-fmt.

@Strum355
Copy link
Contributor

Strum355 commented May 2, 2023

Huh its still different, even though rnix-lsp supposedly runs nixpkgs-fmt 🤔 im gonna have to look into that

@keegancsmith
Copy link
Member Author

Maybe I am using a different version of nixpkgs-fmt? nixpkgs-fmt 1.3.0

@Strum355
Copy link
Contributor

Strum355 commented May 3, 2023

Yup, looks like rnix-lsp is still on 1.2.0. I agree with lilyball that I prefer the output from 1.2.0, but alas 😅

@Strum355
Copy link
Contributor

Strum355 commented May 3, 2023

Assuming all nix files are formatted with nixpkgs-fmt 1.3.0, then Im cool with merging this. I can sort out lsp using the wrong formatter on my own end

@keegancsmith
Copy link
Member Author

@Strum355 I under-engineered supporting nix fmt. Approve? :)

@Strum355
Copy link
Contributor

Strum355 commented May 3, 2023

@Strum355 I under-engineered supporting nix fmt. Approve? :)

Already ahead of you :smugcat: https://github.com/sourcegraph/sourcegraph/pull/51409

@keegancsmith keegancsmith enabled auto-merge (squash) May 3, 2023 14:21
auto-merge was automatically disabled May 4, 2023 11:56

Pull request was closed

@keegancsmith keegancsmith deleted the k/nix-format branch May 4, 2023 11:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed nix For Nix(OS) related issues/PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants